FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Versioning directives policy by story645 · Pull Request #24161 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Versioning directives policy - #24161

Merged
QuLogic merged 1 commit into
matplotlib:mainfrom
story645:versioning-directives-policy
Oct 26, 2022
ConversationCommits (1)ChecksFiles changed
Merged

Versioning directives policy#24161
QuLogic merged 1 commit into
matplotlib:mainfrom
story645:versioning-directives-policy

Conversation

story645 commented Oct 14, 2022

Copy link
Copy Markdown
Member

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

story645 mentioned this pull request Oct 14, 2022
Merged
story645 mentioned this pull request Oct 14, 2022
Closed
timhoffm reviewed Oct 15, 2022
View reviewed changes
Comment thread .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.

timhoffm Oct 15, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This misses now a general statement "New features are documented".

story645 Oct 15, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yeah cause I wasn't sure how they're documented besides docstrings, what's new, and examples?

Comment thread .github/PULL_REQUEST_TEMPLATE.md Outdated
Comment thread .github/PULL_REQUEST_TEMPLATE.md Outdated
Comment thread .github/PULL_REQUEST_TEMPLATE.md Outdated
Comment thread doc/devel/coding_guide.rst Outdated

tacaswell commented Oct 15, 2022

Copy link
Copy Markdown
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 commented Oct 15, 2022

Copy link
Copy Markdown
Member

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

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

oscargus commented Oct 16, 2022

Copy link
Copy Markdown
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 commented Oct 16, 2022

Copy link
Copy Markdown
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 commented Oct 17, 2022

Copy link
Copy Markdown
Member

Agreed. Missed that comment though...

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

timhoffm commented Oct 21, 2022

Copy link
Copy Markdown
Member

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

story645 requested a review from QuLogic October 21, 2022 04:00

story645 commented Oct 21, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

@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 commented Oct 21, 2022

Copy link
Copy Markdown
Member

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

QuLogic reviewed Oct 22, 2022
View reviewed changes
Comment thread doc/devel/coding_guide.rst Outdated
Comment thread doc/devel/coding_guide.rst Outdated

story645 commented Oct 23, 2022

Copy link
Copy Markdown
Member Author

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

PR template: broke release info out into seperate section and tweaked…
8454605 
… 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 force-pushed the versioning-directives-policy branch from d3d3fd6 to 8454605 Compare October 23, 2022 19:45

timhoffm commented Oct 24, 2022

Copy link
Copy Markdown
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 commented Oct 24, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

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 commented Oct 26, 2022

Copy link
Copy Markdown
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 merged commit 42fdbea into matplotlib:main Oct 26, 2022
story645 deleted the versioning-directives-policy branch October 26, 2022 06:03
story645 linked an issue Oct 27, 2022 that may be closed by this pull request
[Doc]: add sphinx versioning directives #23506
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

QuLogic QuLogic approved these changes
timhoffm timhoffm approved these changes

Assignees

No one assigned

Labels

Documentation Maintenance

Projects

None yet

Milestone

v3.7.0

Development

Successfully merging this pull request may close these issues.

[Doc]: add sphinx versioning directives

6 participants

Back | FazBrowse Home | New Git URL