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

argument naming consistency #99

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

supakeen
Copy link
Member

This implements my ideas from #98. Each change is a separate commit so they can be dropped or modified.

@mvo5
Copy link
Collaborator

mvo5 commented Jan 27, 2025

Thanks for the PR. This is a tricky one. I agree with the concerns. Its also a bit of a can of worms:

  1. The --datadir is a bit annyoing, the only thing we currently can customize here is the repositories and its only called like this because the reporegistry.New() will always append repositories/to the dirs passed into reporegistry.New(). So we could fix reporegistry.New() to take (the direct repo config paths) and then we could rename this to --repositories. But in the future with the image defintion in yaml this will most likely point to those as well so probably worth keeping. I not super convinved by --defs though, its nice and short but lacks the hint of that its a "rootdir" for a bunch of data.
  2. The --output feels a bit too generic. It could mean, file, dir or format (like we use it now). As we generally only support specifying a dir having "dir" in it seems okay. We will (hopefully) also get supprot to change the output-filenamename of the imagefile itself (instead of the current hardcoding) and then this might also become a bit confusing.
  3. Using "format" instead of "output" sounds nice, I think its early enough to do it and it seems cleaner to me (and its early enough to change this still I feel)
  4. blueprint vs blueprint-path is a tricky one too, the later is better in the "more-precise" sense, the former is shorter and the help explain it but maybe not enough? I slightly lean towards shorter.

Sorry, many thoughts, naming is hard.

@achilleas-k achilleas-k self-requested a review January 28, 2025 14:44
Part of naming our arguments consistently.

Signed-off-by: Simon de Vlieger <[email protected]>
For consistency in argument naming.

Signed-off-by: Simon de Vlieger <[email protected]>
Rename the named argument for consistency with other arguments.

Signed-off-by: Simon de Vlieger <[email protected]>
@supakeen supakeen force-pushed the argument-consistency branch from 2043c71 to b4009a2 Compare January 29, 2025 21:54
@achilleas-k
Copy link
Member

Not sure about --defs. We use it a lot internally, but I have no idea how (image/distro) "definitions" sounds to a user, if it sounds like what it is.

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