-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Versioning directives policy #24161
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
Versioning directives policy #24161
Conversation
.github/PULL_REQUEST_TEMPLATE.md
Outdated
- [ ] New features are documented, with examples if plot related. | ||
- [ ] New features have an entry in `doc/users/next_whats_new/` (follow instructions in README.rst there). | ||
- [ ] API changes documented in `doc/api/next_api_changes/` (follow instructions in README.rst there). | ||
- [ ] New plotting related features are documented with examples. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This misses now a general statement "New features are documented".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah cause I wasn't sure how they're documented besides docstrings, what's new, and examples?
Responding to a question from @jklymak in #24160, I think we should be on the liberal side of adding these notes. It seems like we get ~ a bug report per month of "Why doesn't this documented feature work?!" to which the answer is "it was added in a newer version than you have installed. If the docs clearly say what version the feature was added in hopefully we can reduce that user frustration. That said, having the version switcher on the docs does help make it easier to see the docs for the version you have installed. If we get to a point where we have so many of these where it is distracting, my knee-jerk thought is to start removing 3-4 version of markers to keep the total number down... |
That's fine with me. Otoh we need to be explicit about what is new if we do this for api changes. |
27d63c1
to
7e79c01
Compare
Should we also have some removal policy? I guess the documents may be a bit unreadable if version info for every change since 2004 would be in there... |
I'd go with Tom's suggestion. We should start thinking of removal, when it get's too distracting. But that can be decided when we're there. Guessing up-front is not necessary. |
Agreed. Missed that comment though... |
7e79c01
to
a883398
Compare
Since this is not only documentation but sets a policy, I'd like to have a second review. |
@story645 I've replied here: #24160 (comment) |
Um should there be a placeholder letter instead of 3 too or not worth it for now? |
… some other wording Coding guide: documented directive policy Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com> Co-authored-by: melissawm <melissawm@gmail.com> Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
d3d3fd6
to
8454605
Compare
This is clear enough. Let's not overthink this. (IMHO people would understand all of |
So leaving it here as 3.N to be consistent but b/f #23941 we were using X.Y. as minor.patch and now I'm tempted to make a follow up PR that just spells these things out as |
I'm not in favor of If anything I have a slight preference for the concrete example
|
Added policy to coding guidelines and PR guidelines based on discussion in #23506, samples of the directives are at #24160