Skip to content

RFC: Unstable APIs #2281

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Apr 5, 2023
Merged

RFC: Unstable APIs #2281

merged 7 commits into from
Apr 5, 2023

Conversation

celinval
Copy link
Contributor

@celinval celinval commented Mar 8, 2023

Description of changes:

Create a RFC for feature gate for APIs included in the Kani library.

Resolved issues:

Related RFC:

Optional #2279

Call-outs:

Testing:

  • How is this change tested?

  • Is this a refactor change?

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@celinval celinval requested a review from a team as a code owner March 8, 2023 02:07
@celinval celinval mentioned this pull request Mar 8, 2023
Merged
4 tasks
@celinval celinval force-pushed the issue-2279-unstable-api branch from 13c579f to 00030c6 Compare March 8, 2023 02:12
adpaco-aws
adpaco-aws reviewed Mar 8, 2023
View reviewed changes
Copy link
Contributor

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

Thanks for writing this, @celinval !

Overall, I agree with the proposal. I've left some comments regarding motivation and other procedures (e.g., a warning) that I'd like you to consider.

@adpaco-aws adpaco-aws added the T-RFC Label RFC PRs and Issues label Mar 8, 2023
adpaco-aws
adpaco-aws reviewed Mar 10, 2023
View reviewed changes
Copy link
Contributor

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

It's looking great!

I'd only extend the "User Impact" section with an explanation on the current approach to unstable options, briefly mentioning its limitations.

zhassan-aws
zhassan-aws approved these changes Mar 10, 2023
View reviewed changes
Copy link
Contributor

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks @celinval!

fzaiser
fzaiser reviewed Mar 13, 2023
View reviewed changes
fzaiser
fzaiser reviewed Mar 13, 2023
View reviewed changes
fzaiser
fzaiser reviewed Mar 13, 2023
View reviewed changes
fzaiser
fzaiser reviewed Mar 13, 2023
View reviewed changes
@fzaiser
Copy link
Contributor

fzaiser commented Mar 13, 2023

Thanks @celinval! I took a look as well as my PR #1659 will be the first API to be affected by this. It sounds good to me, I only added some minor comments (mostly typos).

@feliperodri feliperodri changed the title RFC for unstable APIs RFC: Unstable APIs Mar 21, 2023
@fzaiser
Copy link
Contributor

fzaiser commented Mar 31, 2023

Could I ask for an update given that #1659 is blocked on this?

@celinval
Copy link
Contributor Author

celinval commented Apr 5, 2023

Thanks @celinval! I took a look as well as my PR #1659 will be the first API to be affected by this. It sounds good to me, I only added some minor comments (mostly typos).

That's awesome! Thanks for the feedback. Sorry for the late reply, I was out for a couple of weeks.

adpaco-aws
adpaco-aws approved these changes Apr 5, 2023
View reviewed changes
Copy link
Contributor

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

Thank you, @celinval !

@celinval @adpaco-aws
Update rfc/src/rfcs/0006-unstable-api.md
ab96858 
Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
@celinval celinval enabled auto-merge (squash) April 5, 2023 18:35
@celinval celinval merged commit 0f0e2b8 into model-checking:main Apr 5, 2023
@celinval celinval mentioned this pull request Apr 13, 2023
Merged
4 tasks
@celinval celinval mentioned this pull request May 3, 2023
Merged
4 tasks
@zhassan-aws zhassan-aws mentioned this pull request Jul 20, 2023
Merged
4 tasks
This was referenced Mar 15, 2025
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhassan-aws zhassan-aws zhassan-aws approved these changes

+2 more reviewers

@fzaiser fzaiser fzaiser left review comments

@adpaco-aws adpaco-aws adpaco-aws approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
T-RFC Label RFC PRs and Issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@celinval @fzaiser @adpaco-aws @zhassan-aws