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

feat: add settings models for bootstrap commands #46

Merged
merged 1 commit into from
Sep 4, 2024
Merged

feat: add settings models for bootstrap commands #46

merged 1 commit into from
Sep 4, 2024

Conversation

piyush-jena
Copy link
Contributor

@piyush-jena piyush-jena commented Jul 31, 2024 •
edited
Loading

Issue #, if available:

Description of changes:
This PR contains the following changes:

  1. settings sdk changes for supporting settings.bootstrap-commands in bottlerocket.
  2. Modified deny.toml to get a successful build. BSD-2 license and multiple versions of crates were allowed. Currently the repository doesn't have a Cargo.lock file which means there is no control over the dependency tree.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

The change in deny.toml will be done in a separate commit after the code review and before merging.

cbgbt
cbgbt reviewed Jul 31, 2024
View reviewed changes
Copy link
Contributor

@cbgbt cbgbt left a comment

Choose a reason for hiding this comment

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

As a tip, you can run make build locally to emulate what the CI will do in this repo. Seems like there are a few changes it wants you to make (cargo fmt).

bottlerocket-settings-models/modeled-types/src/shared.rs Outdated Show resolved Hide resolved
bottlerocket-settings-models/settings-extensions/bootstrap-commands/src/lib.rs Show resolved Hide resolved
@piyush-jena piyush-jena mentioned this pull request Aug 1, 2024
Merged
@piyush-jena piyush-jena force-pushed the develop branch 4 times, most recently from 77be984 to 561528b Compare August 6, 2024 17:13
cbgbt
cbgbt approved these changes Aug 6, 2024
View reviewed changes
Copy link
Contributor

@cbgbt cbgbt left a comment

Choose a reason for hiding this comment

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

Nice work!

bottlerocket-settings-models/modeled-types/src/shared.rs Outdated Show resolved Hide resolved
bottlerocket-settings-models/modeled-types/src/shared.rs Outdated Show resolved Hide resolved
@cbgbt cbgbt mentioned this pull request Aug 6, 2024
Merged
ytsssun
ytsssun reviewed Aug 7, 2024
View reviewed changes
deny.toml Outdated Show resolved Hide resolved
@piyush-jena piyush-jena force-pushed the develop branch 2 times, most recently from f52f12e to 3fcb025 Compare August 8, 2024 01:01
bcressey
bcressey reviewed Aug 8, 2024
View reviewed changes
deny.toml Outdated Show resolved Hide resolved
deny.toml Outdated Show resolved Hide resolved
bottlerocket-settings-models/modeled-types/src/shared.rs Outdated Show resolved Hide resolved
bottlerocket-settings-models/modeled-types/src/shared.rs Outdated Show resolved Hide resolved
bottlerocket-settings-models/modeled-types/src/shared.rs Outdated Show resolved Hide resolved
bottlerocket-settings-models/modeled-types/src/shared.rs Outdated Show resolved Hide resolved
bottlerocket-settings-models/settings-extensions/bootstrap-commands/src/lib.rs Outdated Show resolved Hide resolved
bottlerocket-settings-models/settings-extensions/bootstrap-commands/src/lib.rs Outdated Show resolved Hide resolved
@piyush-jena piyush-jena force-pushed the develop branch 5 times, most recently from 25fc51d to a91bd13 Compare August 12, 2024 10:02
bcressey
bcressey reviewed Aug 12, 2024
View reviewed changes
Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

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

Looks pretty close. If you rebase on develop, will that fix the build error?

bottlerocket-settings-models/modeled-types/src/shared.rs Show resolved Hide resolved
bottlerocket-settings-models/settings-extensions/bootstrap-commands/src/lib.rs Show resolved Hide resolved
@piyush-jena piyush-jena force-pushed the develop branch 8 times, most recently from 83b6502 to 322abd2 Compare August 14, 2024 22:03
@stevehipwell stevehipwell mentioned this pull request Aug 15, 2024
Open
@piyush-jena piyush-jena force-pushed the develop branch 7 times, most recently from 329e1e7 to b2e00ad Compare August 19, 2024 02:05
@piyush-jena piyush-jena force-pushed the develop branch 5 times, most recently from fba416f to c1409e9 Compare August 27, 2024 20:23
cbgbt
cbgbt approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@cbgbt cbgbt left a comment

Choose a reason for hiding this comment

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

Nice work!

@piyush-jena piyush-jena force-pushed the develop branch 2 times, most recently from 27d1e78 to d5e6e6b Compare August 28, 2024 20:11
@piyush-jena piyush-jena changed the title Settings and Models for Bootstrap Commands feat: add settings models for bootstrap commands Aug 28, 2024
@cbgbt cbgbt mentioned this pull request Sep 4, 2024
Merged
@piyush-jena piyush-jena merged commit d1eee00 into bottlerocket-os:develop Sep 4, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ytsssun ytsssun ytsssun left review comments

@yeazelm yeazelm yeazelm left review comments

@arnaldo2792 arnaldo2792 arnaldo2792 left review comments

@cbgbt cbgbt cbgbt approved these changes

@bcressey bcressey bcressey approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@piyush-jena @bcressey @cbgbt @arnaldo2792 @yeazelm @ytsssun