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
feat: add command in charmil cli #207
Conversation
Very nice! |
The add command fails when any of the parent directories in Logs:>> ./charmil add --cmdName="mycmd" --cmdPath="./cmd"
Usage:
charmil add [flags]
Examples:
$ charmil add cliname
Flags:
-s, --cmdName string name of the new command
-c, --cmdPath string command path where you want to create the command (default ".")
-h, --help help for add
mkdir cmd/mycmd: no such file or directory I had a similar issue with the crud command. Mentioning the PR which fixed it (for reference): #192 Just replacing |
|
||
// Adds local flags | ||
cmd.Flags().StringVarP(&tmplData.CmdPath, f.Localizer.LocalizeByID("add.flag.cmdPath.name"), "c", ".", f.Localizer.LocalizeByID("add.flag.cmdPath.description")) | ||
cmd.Flags().StringVarP(&tmplData.CmdName, f.Localizer.LocalizeByID("add.flag.cmdName.name"), "s", "", f.Localizer.LocalizeByID("add.flag.cmdName.description")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can add a flag here that takes a target path for the language file and then generate the file in that location.
The crud command does this too. Check this out for reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, we can leave it? it would be difficult for the user to give two paths. It's just a file, user can move it wherever they wants manually.
it will be better to generate the locales file with the command file. to let the user figure out what really happened.
cli/internal/template/add/tmpl.go
Outdated
|
||
import "embed" | ||
|
||
// CrudTemplates stores embedded contents of all the add template files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// CrudTemplates stores embedded contents of all the add template files | |
// AddTemplates stores embedded contents of all the add template files |
@@ -0,0 +1,25 @@ | |||
add.cmd.use: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is outside the scope of this PR, but should we localize the use of command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am not sure about this. It's being done in rhoas as well.
should we merge this @wtrocki ? |
Ok.. Rush doesn't help. Release needed. |
Closes #166
Description
Type of change
Checklist