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.

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

chore(ci): Allow releases to have additional feature flags #2405

Merged
merged 4 commits into from
Aug 29, 2023
Merged

chore(ci): Allow releases to have additional feature flags #2405

merged 4 commits into from
Aug 29, 2023

Conversation

Maddiaa0
Copy link
Member

Description

Adds the ability to trigger releases with feature flags.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@Maddiaa0 Maddiaa0 marked this pull request as ready for review August 22, 2023 18:48
@Maddiaa0 Maddiaa0 requested a review from phated August 22, 2023 18:48
@Maddiaa0 Maddiaa0 mentioned this pull request Aug 22, 2023
Merged
5 tasks
phated
phated previously requested changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

I believe cargo requires --features be specified before each feature on the command line. Additionally, I think we need to append the feature flags to the binary being produced so someone doesn't override the binary on a tag with some feature flagged version

@Maddiaa0 Maddiaa0 mentioned this pull request Aug 23, 2023
Closed
@Maddiaa0
Copy link
Member Author

I believe cargo requires --features be specified before each feature on the command line.

Ah you are correct it does, my mistake. Shall I just alter the name of the input to be additional flags, then the complex flag commands can be added by hand. Otherwise i can alter the script to preprocess the flags.

Additionally, I think we need to append the feature flags to the binary being produced so someone doesn't override the binary on a tag with some feature flagged version

This is the current goal was that this would override the current aztec flag, making the UX nice such that a dev can just run noirup -v aztec. However I can also do that to protect against footguns.

@TomAFrench TomAFrench changed the title feat(release): Allow releases to have additional feature flags feat: Allow releases to have additional feature flags Aug 23, 2023
@TomAFrench TomAFrench changed the title feat: Allow releases to have additional feature flags chore(ci): Allow releases to have additional feature flags Aug 23, 2023
kevaundray
kevaundray previously approved these changes Aug 29, 2023
View reviewed changes
@kevaundray kevaundray dismissed phated’s stale review August 29, 2023 10:53

fixed the feature flag issue, it is desired to override binaries so that aztec-cli can use the feature flagged binary version

@kevaundray kevaundray added this pull request to the merge queue Aug 29, 2023
Merged via the queue into master with commit b1b45b5 Aug 29, 2023
16 checks passed
@kevaundray kevaundray deleted the md/feature-flag-release branch August 29, 2023 11:27
TomAFrench added a commit that referenced this pull request Aug 30, 2023
@TomAFrench
Merge branch 'master' into kw/bb-binaries
47f2ab6 
* master: (42 commits)
  fix(ssa): Handle right shift with constants (#2481)
  chore(noir): Release 0.10.4 (#2354)
  fix: Divide by zero should fail to satisfy constraints for `Field` and ints (#2475)
  fix(ssa): Remove padding from ToRadix call with constant inputs (#2479)
  fix: Implement handling of array aliasing in the mem2reg optimization pass (#2463)
  chore: resolve `Instruction` inputs fully before checking against cache (#2472)
  chore: Move independent `run_test` function into nargo core (#2468)
  feat: Standard library functions can now be called with closure args  (#2471)
  feat(frontend): aztec syntactic sugar (feature flagged) (#2403)
  chore(ci): enforce compliance with `cargo fmt` (#2467)
  chore(ci): Allow releases to have additional feature flags (#2405)
  feat: Add `assert_eq` keyword (#2137)
  fix(ssa): Do not optimize for allocates in constant folding (#2466)
  feat(ssa): Reuse existing results for duplicated instructions with no side-effects (#2460)
  fix: Closure lvalue capture bugfix (#2457)
  feat: Syntax for environment types now works with generics (#2383)
  fix(parser): fixes for the parsing of 'where' clauses (#2430)
  fix: Run `wasm` nodejs tests with no fails (#2387)
  chore: Run `cargo fmt` (#2455)
  chore: Perform formatting changes to integration tests (#2448)
  ...
TomAFrench added a commit that referenced this pull request Aug 30, 2023
@TomAFrench
Merge branch 'master' into remove-keys-from-artifact
b04ed6b 
* master: (42 commits)
  fix(ssa): Handle right shift with constants (#2481)
  chore(noir): Release 0.10.4 (#2354)
  fix: Divide by zero should fail to satisfy constraints for `Field` and ints (#2475)
  fix(ssa): Remove padding from ToRadix call with constant inputs (#2479)
  fix: Implement handling of array aliasing in the mem2reg optimization pass (#2463)
  chore: resolve `Instruction` inputs fully before checking against cache (#2472)
  chore: Move independent `run_test` function into nargo core (#2468)
  feat: Standard library functions can now be called with closure args  (#2471)
  feat(frontend): aztec syntactic sugar (feature flagged) (#2403)
  chore(ci): enforce compliance with `cargo fmt` (#2467)
  chore(ci): Allow releases to have additional feature flags (#2405)
  feat: Add `assert_eq` keyword (#2137)
  fix(ssa): Do not optimize for allocates in constant folding (#2466)
  feat(ssa): Reuse existing results for duplicated instructions with no side-effects (#2460)
  fix: Closure lvalue capture bugfix (#2457)
  feat: Syntax for environment types now works with generics (#2383)
  fix(parser): fixes for the parsing of 'where' clauses (#2430)
  fix: Run `wasm` nodejs tests with no fails (#2387)
  chore: Run `cargo fmt` (#2455)
  chore: Perform formatting changes to integration tests (#2448)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevaundray kevaundray kevaundray approved these changes

@phated phated Awaiting requested review from phated

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Maddiaa0 @phated @kevaundray