Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat(agent/agentcontainers): add feature options as envs#18576

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

mafredri
Copy link
Member

@mafredrimafredri commentedJun 25, 2025
edited
Loading

Sincedevcontainer-feature.json does not support referencing options anywhere outside the feature installation, especially within the JSON itself, this change implements a work-around by defining environment variables based on the feature options that we can use in our features.

This allows us to define a dynamic URL for e.g. thecode-server feature.

@mafredrimafredri requested review fromjohnstcn andDanielleMaywood and removed request forjohnstcnJune 25, 2025 14:14
@mafredrimafredri marked this pull request as ready for reviewJune 25, 2025 14:14
Comment on lines 41 to 52
// OptionsAsEnvs converts the DevcontainerFeatures into a list of
// environment variables that can be used to set feature options.
// The format is FEATURE_<FEATURE_NAME>_<OPTION_NAME>=<value>.
// For example, if the feature is:
//
//"ghcr.io/coder/devcontainer-features/code-server:1": {
// "port": 9090,
// }
//
// It will produce:
//
//FEATURE_CODE_SERVER_PORT=9090
Copy link
Contributor

Choose a reason for hiding this comment

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

I can foresee an issuepotentially arising with this approach.

"go": {  "server-port": 9090},"go-server": {  "port": 9091}

Would both produceFEATURE_GO_SERVER_PORT but they have different values

Copy link
Member

@johnstcnjohnstcnJun 25, 2025
edited
Loading

Choose a reason for hiding this comment

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

yeah we might want something likeFEATURE__GO_SERVER__PORT /FEATURE__GO__SERVER_PORT

edit: jinx 😄

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Good call-out, not sure how we should fix though. Use__ as separator? Include the:1 as_1?

Copy link
Contributor

@DanielleMaywoodDanielleMaywoodJun 25, 2025
edited
Loading

Choose a reason for hiding this comment

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

I think__ could work as a separator, it might still have the same issue if people want to double up on their- but I think that is unlikely.

Copy link
MemberAuthor

@mafredrimafredriJun 25, 2025
edited
Loading

Choose a reason for hiding this comment

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

Seems we had the same idea@johnstcn, let's go with__ although using_OPTION_ might also work. LikeFEATURE_GO_SERVER_OPTION_PORT. The risk of collision is so small that if it does happen, somebody can think about why they chose to call an option option.

Copy link
Member

@johnstcnjohnstcnJun 25, 2025
edited
Loading

Choose a reason for hiding this comment

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

I think double-underscore separator should work fine.

edit: jinx again, but I do like your OPTION_OPTION as well!

mafredri reacted with laugh emoji
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Unless you guys are greatly opposed I'd actually like to go with the_OPTION_ approach. Seems more human friendly.

johnstcn reacted with thumbs up emoji
Copy link
Contributor

Choose a reason for hiding this comment

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

@mafredri I prefer your suggestion, go with_OPTION_.

johnstcn and mafredri reacted with thumbs up emoji
@mafredrimafredrienabled auto-merge (squash)June 25, 2025 14:39
@mafredrimafredri merged commit3c4d920 intomainJun 25, 2025
38 checks passed
@mafredrimafredri deleted the mafredri/feat-agent-agentcontainers-add-feature-options-as-env branchJune 25, 2025 14:41
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsJun 25, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@johnstcnjohnstcnjohnstcn approved these changes

@DanielleMaywoodDanielleMaywoodDanielleMaywood approved these changes

Assignees

@mafredrimafredri

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@mafredri@johnstcn@DanielleMaywood

[8]ページ先頭

©2009-2025 Movatter.jp