-
Notifications
You must be signed in to change notification settings - Fork 160
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 _builtin suffix to builtin names. #1005
Conversation
Benchmark Results for unmodified programs 🚀
|
Codecov Report
@@ Coverage Diff @@
## main #1005 +/- ##
=======================================
Coverage 98.02% 98.02%
=======================================
Files 76 76
Lines 31621 31621
=======================================
Hits 30997 30997
Misses 624 624
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
CHANGELOG.md
Outdated
@@ -2,6 +2,8 @@ | |||
|
|||
#### Upcoming Changes | |||
|
|||
* Add _builtin suffix to builtin names e.g.: output -> output_builtin [#1005](https://github.com/lambdaclass/cairo-rs/pull/1005) |
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.
It should mention it's (technically) a breaking change. Any code not expecting the suffix would break.
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.
Done!
4307125
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.
Minor change to the CHANGELOG needed.
Also, I know it's not documented or enforced anywhere (yet), but we're planning on using conventional commits so updating the CHANGELOG could be automated or at least assisted by the CI, so I would appreciate if we could start following those.
Thanks for the remainder @Oppen ! |
No worries. Do note it's not a blocker. Just if you need to change something else, seize the opportunity. Until documented/enforced by CI I won't consider such thing a must because it would be unfair to reject PRs based on something we didn't tell was needed :) |
* Add _builtin suffix to builtin names. * fix unit tests * Update CHANGELOG.md * Update CHANGELOG.md --------- Co-authored-by: Pedro Fontana <[email protected]>
TITLE
Description
Description of the pull request changes and motivation.
Checklist