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

Add --alias flag to incus-simplestreams #876

Closed
wants to merge 1 commit into from

Conversation

melato
Copy link
Contributor

@melato melato commented May 17, 2024

Add an --alias flag to the command incus-simplestreams add See issue #875

@stgraber
Copy link
Member

Can you edit your commit to include the Signed-off-by: Firstname Lastname <email> line we require (see CONTRIBUTING.md).

From a usability standpoint, I think we should instead have:

  • --no-default-alias to turn off the default distro/release/variant alias
  • --alias to add aliases (can be passed multiple times)

@stgraber stgraber changed the title added incus-simplestreams add --alias flag. #875. Add --alias flag to incus-simplestreams May 21, 2024
@melato
Copy link
Contributor Author

melato commented May 21, 2024

Ok, will submit a new pull request with --no-default-alias, and adding multiple aliases.
--no-default-alias will allow setting no alias at all.
There is an inconsistency with the "incus publish" command, which does not set a default alias.
incus-simplestreams add is another way of publishing.

By the way, why do some flag usages have "" at the end? For example, in cmd/incus/publish.go: cmd.Flags().StringArrayVar(&c.flagAliases, "alias", nil, i18n.G("New alias to define at target")+"")
It does not show up in the usage. Does it indicate something to spf13/cobra?

@stgraber
Copy link
Member

There is an inconsistency with the "incus publish" command, which does not set a default alias.
incus-simplestreams add is another way of publishing.

That's why, those are two different commands so them behaving differently doesn't really bother me. We'd probably have had incus publish also put in a default alias if we had known what we were publishing and if the native incus image store was capable of handling multiple images having the same alias.

By the way, why do some flag usages have "" at the end? For example, in cmd/incus/publish.go: cmd.Flags().StringArrayVar(&c.flagAliases, "alias", nil, i18n.G("New alias to define at target")+"")

I don't recall exactly what issue that's working around other than it being a cobra behavior around default values which we didn't want and that this syntax makes it avoid.

@melato
Copy link
Contributor Author

melato commented May 23, 2024

Closing this pull request. I will create a new one. I'm learning about pull requests and fork branches.

@melato melato closed this May 23, 2024
@melato melato deleted the incus-simplestreams-add-alias branch May 23, 2024 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants