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.

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

simple config command just for setting default registry #100

Merged
merged 8 commits into from
Oct 1, 2024

Conversation

calvinrp
Copy link
Collaborator

@calvinrp calvinrp commented Oct 1, 2024

Just a bare minimum config subcommand for setting the default registry.

wkg config prints current default registry
wkg config --default-registry {REGISTRY} sets the default registry
wkg config --edit launches editor ($EDITOR env var) with the config file

Can follow up with an addition PR for other config options.

@lann
Copy link
Collaborator

lann commented Oct 1, 2024

I'd maybe make the flag name a little more explicit, e.g. --default-registry

@lann
Copy link
Collaborator

lann commented Oct 1, 2024

Two possible follow-up features that I think would have a good bang-for-buck:

  • wkg config: print the current config
    • probably need some #[serde(skip_serializing_if)] on the ConfigToml to make this digestible.
  • wkg config --edit: open the global config file in $EDITOR

@calvinrp
Copy link
Collaborator Author

calvinrp commented Oct 1, 2024

Thanks for the recommendations, @lann . I will update shortly.

@calvinrp
Copy link
Collaborator Author

calvinrp commented Oct 1, 2024

Two possible follow-up features that I think would have a good bang-for-buck:

  • wkg config: print the current config

    • probably need some #[serde(skip_serializing_if)] on the ConfigToml to make this digestible.
  • wkg config --edit: open the global config file in $EDITOR

Added wkg config --edit to launch editor.

Copy link
Collaborator

@thomastaylor312 thomastaylor312 left a comment

Choose a reason for hiding this comment

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

All good to go from my side. Mostly comments/nits that don't necessarily need to be addressed before merge

crates/wkg/src/main.rs Show resolved Hide resolved
crates/wkg/src/main.rs Outdated Show resolved Hide resolved
crates/wkg/src/main.rs Show resolved Hide resolved
Copy link
Collaborator

@lann lann left a comment

Choose a reason for hiding this comment

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

Shoot - forgot to submit review. Not super critical but these comments do show up in --help.

crates/wkg/src/main.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@lann lann left a comment

Choose a reason for hiding this comment

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

Thanks!

@calvinrp calvinrp merged commit 654097b into main Oct 1, 2024
6 checks passed
@calvinrp calvinrp deleted the simple-config branch October 1, 2024 19:22
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.

3 participants