-
Notifications
You must be signed in to change notification settings - Fork 6k
Add a Stability policy #3622
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 a Stability policy #3622
Conversation
harshil21
left a comment
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.
I'm okay with what the policy is covering, can't think of anything we didn't cover yet.
* note on vX.Y vs vX.Y.0
Poolitzer
left a comment
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.
I dont think we fully close #3590 with this. We don't have a standardised approach for moving arguments around, or optional/required ones.
docs/source/stability_policy.rst
Outdated
| Development Versions | ||
| ~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| Before a feature is in a release, it is not covered by this policy and may change. |
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.
| Before a feature is in a release, it is not covered by this policy and may change. | |
| Before a feature is in a release, it is not covered by this policy and may change drastically between versions. |
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.
no, this is about code on master branch that's not been released yet
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.
or about stuff in not stable releases no?
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.
No. non-stable releases are still releases. What I mean here is that code that hasn't made it to pypi can change at any time.
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.
Wait but this doesn't make sense with the next sentence then. Since pre-releases are also not covered, can't you merge it in one sentence?
Before a feature is in a stable release, i.e. the feature was merged into the master branch but not released yet or it is only present in pre-releases marked as alpha, beta or release candidate, it is not covered by this policy and may change at any time.
You are right, I did not write down explicit guidelines for that. While writing the draft I had the impression that I might be better for the dev team if we left that out and instead had a paragraph that gives us some leeway to make these decision depending on the specific use case and also the teams current state of availability. That's what I tried to achieve by
OTOH I do see that this could backfire in the sense that we do have repeated discussions every time after all. What would you think about listing "common cases that might occur" in addition to above paragraph - along with notes about which is generally preferred or "the team will decide depending on their capacity" or smth like that? |
|
Or maybe we leave it like it is right now, and just write us an "Internal" document e.g. in the wiki one can always consult when doing an API update. |
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.
Some usual nitpicks. Also, our stability policy has no stability policy over the use of UK or US spelling (behavior vs. behaviour). Or maybe it does and Pool's comments don't 😄
I like that. Including this in the policy would not be needed since it comes under "trade-off between backwards compatibility and fully complying with the Bot API" imo |
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.
I'm happy with the policy, just last few comments:
lemontree210
left a comment
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.
tons of nitpicks and one hopefully meaningful question
Co-authored-by: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com>
Poolitzer
left a comment
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.
Small comments
|
Thank for the review @Poolitzer , I made according changes. There still a number of unresolved threads from your previous review (#3622 (review)). Could you look through them and check if those can be closed from your pov? |
Closes #3590
Opening a PR as discussion of the contents is easier by leaving PR reviews :)
Also moved the PR template to the contrib guide - that's not directly related but IIRC it came in the same discussion in the dev chat
python-telegram-botin the stability policy