Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added build cli options proposal #328

Merged
merged 3 commits into from
Jan 10, 2024

Conversation

Roemer
Copy link
Contributor

@Roemer Roemer commented Oct 31, 2023

Added a simple proposal for the spec change in #324

@Roemer Roemer requested a review from a team as a code owner October 31, 2023 13:53
@Roemer Roemer mentioned this pull request Oct 31, 2023
Copy link
Member

@samruddhikhandale samruddhikhandale 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 for taking the time in contributing this proposal, we appreciate it 👏

Copy link
Contributor

@chrmarti chrmarti 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 the proposal! Left a few questions.

@Roemer Roemer force-pushed the proposal/build-cli-options branch from ba2a264 to a92b465 Compare November 1, 2023 10:20
@Roemer
Copy link
Contributor Author

Roemer commented Nov 7, 2023

I just stumpled uppon problems with features as well. Should this proposal also add a point that those options should also be added when adding features? I am not totally sure how features work. Are they processed in separate build commands? If so, then the options should just be passed there as well.

@Roemer Roemer requested a review from chrmarti November 7, 2023 09:08
@chrmarti
Copy link
Contributor

chrmarti commented Dec 1, 2023

The image is built in a single docker build run, adding build args to features might allow for conflicting arguments.

Copy link
Contributor

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@eljog eljog left a comment

Choose a reason for hiding this comment

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

Co-authored-by: Eljo George <eljog@github.com>
@chrmarti chrmarti merged commit 299158f into devcontainers:main Jan 10, 2024
1 check passed
chrmarti added a commit that referenced this pull request Jan 15, 2024
@chrmarti chrmarti mentioned this pull request Jan 15, 2024
@Roemer Roemer deleted the proposal/build-cli-options branch January 15, 2024 15:08
Copy link

@113ll3 113ll3 left a comment

Choose a reason for hiding this comment

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

Looks like a buzz

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants