Skip to content

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

Merged
merged 1 commit into from
Oct 26, 2022

Conversation

story645
Copy link
Member

Added policy to coding guidelines and PR guidelines based on discussion in #23506, samples of the directives are at #24160

- [ ] 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.
Copy link
Member

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".

Copy link
Member Author

@story645 story645 Oct 15, 2022

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?

@tacaswell
Copy link
Member

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...

@jklymak
Copy link
Member

jklymak commented Oct 15, 2022

That's fine with me. Otoh we need to be explicit about what is new if we do this for api changes.

@tacaswell tacaswell added this to the v3.7.0 milestone Oct 15, 2022
@story645 story645 force-pushed the versioning-directives-policy branch 2 times, most recently from 27d63c1 to 7e79c01 Compare October 16, 2022 01:30
@oscargus
Copy link
Member

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...

@timhoffm
Copy link
Member

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.

@oscargus
Copy link
Member

Agreed. Missed that comment though...

@story645 story645 force-pushed the versioning-directives-policy branch from 7e79c01 to a883398 Compare October 20, 2022 19:52
@timhoffm
Copy link
Member

Since this is not only documentation but sets a policy, I'd like to have a second review.

@story645 story645 requested a review from QuLogic October 21, 2022 04:00
@story645
Copy link
Member Author

story645 commented Oct 21, 2022

@timhoffm question from #24160 - do we want to explicitly state whether the directives should be blank or give some context or leave that as a case by case?
ETA: Also thanks!

@timhoffm
Copy link
Member

@story645 I've replied here: #24160 (comment)

@story645
Copy link
Member Author

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>
@story645 story645 force-pushed the versioning-directives-policy branch from d3d3fd6 to 8454605 Compare October 23, 2022 19:45
@timhoffm
Copy link
Member

Um should there be a placeholder letter instead of 3 too or not worth it for now?

This is clear enough. Let's not overthink this. (IMHO people would understand all of X.Y, 3.N, 3.6)

@story645
Copy link
Member Author

story645 commented Oct 24, 2022

IMHO people would understand all of X.Y, 3.N, 3.6

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 major.minor.patch throughout the dev docs

@timhoffm
Copy link
Member

I'm not in favor of major.minor.patch because that is quite verbose and it's less clear that these are variables.

If anything I have a slight preference for the concrete example .. versionadded:: 3.6. - People are able to generalize from that and won't copy literally. Generalizing seems a bit simpler that guessing what a variable should be. But as said

Let's not overthink this. (IMHO people would understand all of X.Y, 3.N, 3.6)

@QuLogic QuLogic merged commit 42fdbea into matplotlib:main Oct 26, 2022
@story645 story645 deleted the versioning-directives-policy branch October 26, 2022 06:03
@story645 story645 linked an issue Oct 27, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Doc]: add sphinx versioning directives
6 participants