-
Notifications
You must be signed in to change notification settings - Fork 339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add notes/comments field to parameters #7456
Comments
I did a PR on this issue, please take a look! |
@ntheanh201 - can you provide a link to the PR? |
it's here: #7845 |
Responding to #7845 (comment) here.
We provide only features. We don't get to decide how users use those features. In your words:
So trying to police how people use ATC hurts ATC and we shouldn't do it. I already know of one company that does not contribute their many ATC additions back to the proect, because their use case for ATC lies outside the scope of changes that ATC typically accepts. They are probably the most successful user of ATC, and not accepting unconventional changes is our loss.
You have already voiced your opinion about how you think things things shoul be organized in your Global Configuration blueprint (#7015), and blocking other people's incremental PRs because you think a different high-effort idea is better hurts the project.
The author of #7845 clearly wants this feature, as do several other ATC users who I have talked to. It sounds like you're against implementing #7456 because of the edge case of a user potentially trying to derive structure out of the comments, but that's not a good enough to block accepting implementation itself. If you're concerned about an edge case like that, you could suggest on the PR adding a note to the field that it should only be used to document the property's purpose, and that using it for a structural reason would be unsound. |
I'm not sure what you're saying. This is a new feature is it not? I'm not objecting on the grounds that it's a breaking change; that would be silly. I've authored many a breaking change.
That's not at all what's happening here. I'm against this idea because I think it would have a negative impact on its own terms. Not because I have some superior idea. If I'd never given Global Configuration a moment's thought, I would still think that comments on Parameters would be a bad idea - which is also kind of a moot point, because I'm not actually sure how those two ideas are related. Global Configuration would remove some dedicated Parameters, they would do nothing to the notion of Parameters overall. I don't see how those are conflicting ideas.
I had guessed that the author of that PR opened it only because this issue exists. He or she has said nothing to indicate any personal attachment to the idea. I also can't speak to the desires of others you've talked to, because they haven't taken part in the discussion. Which is what this is, ultimately. I'd be more than happy to take this to the mailing list. But this is kind of totally standard procedure. Somebody proposed a change, I have concerns that it would ultimately be a net negative impact, and I voiced them. Then nobody else addressed - or even acknowledged - those concerns. So to somehow accuse me of standing in the way of something everyone wants is pretty strange to me. Requiring consensus on changes to the project isn't my dastardly plan to impede some specific PR, it's a founding principle of the ASF and older by far than this project. I'm just taking part, is all. |
I could be wrong, but from the implementer's contribution history, I got the idea that they are using ATC in some capacity. So it stands to reason that, if they opened a PR, they would find practical benefit to the feature being implemented. In some cases, though maybe not this one, a contributor is even using the feature they are PRing before it is even merged.
Sure
To be clear, I don't take issue with anything you've said, only that you -1ed the Issue on a contributor's PR.
Agreed, but IMO special care should be taken in cases when -1ing an Issue only after a contributor has submitted a PR for it, and that includes not -1ing the Issue on that PR. The will of users of ATC to give back to the project is often fragile and contributors are easily dissuaded, so we need to do what we can to instead encourage their contributions, even in cases where discussion on an Issue means that a contributor's corresponding PR is not likely to be accepted. |
Actually, I'm (we’re) using ATC to develop a CDN. I took this issue to implement because I see that we need it, we need more context of each parameter’s objective and why we need it. It's not a kinda high impact issue but IMO it's nice to have. So that's why I contribute back to the community, and I also wanna become a long-term contributor to this project |
Thank you for clarifying.
You're well on your way. |
This Improvement request (usability, performance, tech debt, etc.) affects these Traffic Control components:
I believe this would just be a TO/TP change. It would be a new field on the TO parameter object that t3c could ignore. The field is for humans.
Current behavior:
Parameters have 4 fields:
there is no field to add comments or notes to explain why a particular value was chosen for a parameter leaving operators to guess why the value was chosen.
New behavior:
Add a 5th field (comments/notes) where humans can add comments regarding the parameter. i.e. why a particular value was chosen.
The text was updated successfully, but these errors were encountered: