Skip to content
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

RFC: Unstable APIs #2281

Merged
merged 7 commits into from
Apr 5, 2023
Merged

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 force-pushed the issue-2279-unstable-api branch from 13c579f to 00030c6 Compare March 8, 2023 02:12
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.

rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
@adpaco-aws adpaco-aws added the T-RFC Label RFC PRs and Issues label Mar 8, 2023
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.

rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Show resolved Hide resolved
rfc/src/rfcs/0006-unstable-api.md Show resolved Hide resolved
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
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.

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 !

rfc/src/rfcs/0006-unstable-api.md Outdated Show resolved Hide resolved
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
4 tasks
@zhassan-aws zhassan-aws mentioned this pull request Jul 20, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-RFC Label RFC PRs and Issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants