Skip to content

Conversation

@Bibo-Joshi
Copy link
Member

@Bibo-Joshi Bibo-Joshi commented Mar 24, 2023

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

  • add a comment about dropping older python versions
  • move wiki section to wiki
  • replace "PTB" with python-telegram-bot in the stability policy

@Bibo-Joshi Bibo-Joshi added enhancement ⚙️ documentation affected functionality: documentation labels Mar 24, 2023
Copy link
Member

@harshil21 harshil21 left a 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.

@Bibo-Joshi Bibo-Joshi requested a review from harshil21 April 7, 2023 19:25
Copy link
Member

@Poolitzer Poolitzer left a 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.

Development Versions
~~~~~~~~~~~~~~~~~~~~

Before a feature is in a release, it is not covered by this policy and may change.
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

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

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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.

@Bibo-Joshi
Copy link
Member Author

I dont think we fully close #3590 with this. We don't have a standardised approach for moving arguments around, or optional/required ones.

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

While we try to make such changes backwards compatible, this is not always possible or only with significant effort.
In such cases we will find a trade-off between backwards compatibility and fully complying with the Bot API, which may result in breaking changes.
We highly recommend using keyword arguments, which can help to make such changes non-breaking on your end.

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?

@Poolitzer
Copy link
Member

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.

Copy link
Member

@lemontree210 lemontree210 left a 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 😄

@harshil21
Copy link
Member

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.

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

Copy link
Member

@harshil21 harshil21 left a 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:

Copy link
Member

@lemontree210 lemontree210 left a 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

Bibo-Joshi and others added 3 commits April 17, 2023 15:49
Co-authored-by: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com>
Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

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

Small comments

@Bibo-Joshi
Copy link
Member Author

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?

@Bibo-Joshi Bibo-Joshi merged commit 1c6ae43 into master May 7, 2023
@Bibo-Joshi Bibo-Joshi deleted the stability-policy branch May 7, 2023 12:50
@github-actions github-actions bot locked and limited conversation to collaborators May 15, 2023
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

⚙️ documentation affected functionality: documentation 🔌 enhancement pr description: enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stability policy/api update guidelines

5 participants