Skip to content

chore: extract option defaults to options package #283

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

Merged
merged 3 commits into from
Jul 29, 2024
Merged

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented Jul 26, 2024

Part of coder/internal#11

Builds on top of #282

More refactoring:

  • Moves DefaultWorkspaceFolder to options package
  • Extracts the option defaults logic to the options package
  • DefaultWorkspaceFolder now returns EmptyWorkspaceDir if GitURL is invalid, meaning it no longer returns an error
  • Extracts osfsWithChmod to its own package

@johnstcn johnstcn self-assigned this Jul 26, 2024
@johnstcn johnstcn changed the title Cj/option defaults chore: extract option defaults to options package Jul 26, 2024
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

LGTM!

// Temporarily removed these from the default settings to prevent conflicts
// between current and legacy environment variables that add default values.
// Once the legacy environment variables are phased out, this can be
// reinstated to the previous default values.
Copy link
Member

Choose a reason for hiding this comment

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

I know this was just moved and logic didn't change, but I wonder what this comment means. 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Related to ENVBUILDER_ env prefix.

Base automatically changed from cj/pkg-constants to main July 29, 2024 16:20
@johnstcn johnstcn force-pushed the cj/option-defaults branch from 4df4433 to fd6ef17 Compare July 29, 2024 16:23
@johnstcn johnstcn merged commit 09ce456 into main Jul 29, 2024
4 checks passed
@johnstcn johnstcn deleted the cj/option-defaults branch July 29, 2024 16:35
johnstcn added a commit that referenced this pull request Jul 31, 2024
Builds on top of #282 and #283:

- Extracts the logic for --get-cached-image to a separate function
- Also pulls out some other common logic shared between Run and RunCacheProbe.
johnstcn added a commit that referenced this pull request Aug 5, 2024
Signed-off-by: Cian Johnston <[email protected]>
(cherry picked from commit 09ce456)
johnstcn added a commit that referenced this pull request Aug 5, 2024
Builds on top of #282 and #283:

- Extracts the logic for --get-cached-image to a separate function
- Also pulls out some other common logic shared between Run and RunCacheProbe.

(cherry picked from commit cacbcb8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants