-
Notifications
You must be signed in to change notification settings - Fork 10
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: project create #115
feat: project create #115
Conversation
* fix/check-remaining-for-mock-asker * fix: typo Co-authored-by: Shannon Lewis <slewis74@gmail.com> Co-authored-by: Shannon Lewis <slewis74@gmail.com>
This pull request has been linked to Shortcut Story #18973: CLI vNext: project create. |
pkg/cmd/project/create/create.go
Outdated
FlagName = "name" | ||
FlagDescription = "description" | ||
FlagLifecycle = "lifecycle" | ||
FlagConfigAsCode = "cac" |
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.
not sure I am happy with name. Suggestions?
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 found when doing release create
that it's referred to as "Version Controlled" in almost all places in the octopus codebase and web portal and other documentation. E.g. you'll see documentation snippets like "for a version controlled project...", so I used that term there.
So, within the CLI there's precedent to call this "versionControlled".
On the other hand, the "brand name" that keeps showing up on twitter/linkedin/customer forums is always Config As Code, so 🤷 ? tag @jbristowe
"github.com/spf13/cobra" | ||
) | ||
|
||
type Dependable interface { |
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 completely understand we're trying to fit within the patterns and idioms of golang... but Dependable
is perhaps too vague. Does this mean it's never going to let us down? 🤣
GenerateAutomationCmd() | ||
} | ||
|
||
type Dependencies struct { |
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.
Perhaps Dependencies
might be a bit of a vague name here? My immediate thought is "which dependencies does this struct represent? who depends on them and why?"
From a quick look, it appears as though this struct is meant to carry services (client, asker) and options (space, noprompt) that are commonly used across many commands? If so, some possible naming suggestions?
ServicesAndOptions
CommandContext
CliCommandContext
CommandCommonData
StandardCommandProperties
CommonCommandProperties
GeneralCommandProps
... something to think about anyway
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.
tag @slewis74
@@ -0,0 +1,57 @@ | |||
package cmd |
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.
q for @domenicsim1
Given that this isn't a command itself, should it go under the cmd
directory structure, or should it go under just pkg or pkg/somethingelse?
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, although it is suppose to be the basis of cmds. Not sure yet, happy to leave it for now.
pkg/cmd/project/create/create.go
Outdated
FlagName = "name" | ||
FlagDescription = "description" | ||
FlagLifecycle = "lifecycle" | ||
FlagConfigAsCode = "cac" |
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 found when doing release create
that it's referred to as "Version Controlled" in almost all places in the octopus codebase and web portal and other documentation. E.g. you'll see documentation snippets like "for a version controlled project...", so I used that term there.
So, within the CLI there's precedent to call this "versionControlled".
On the other hand, the "brand name" that keeps showing up on twitter/linkedin/customer forums is always Config As Code, so 🤷 ? tag @jbristowe
pkg/cmd/project/create/create.go
Outdated
}, &opts.GitPassword.Value, survey.WithValidator(survey.ComposeValidators( | ||
survey.MaxLength(200), | ||
survey.MinLength(1), | ||
survey.Required, |
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.
IIRC both Required
and MinLength(1)
do the same job, and we probably don't need both of them.
I ran into this when selecting environments for release deployment, which is a multi-select with min-items(1); Required did the same job but produced a better error message so I used that instead
Tidy up various parts, rename flag
updated descriptions and removed short flags for git
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.
🏆 👍
@@ -0,0 +1,57 @@ | |||
package cmd |
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, although it is suppose to be the basis of cmds. Not sure yet, happy to leave it for now.
testutil.NewConfirmPrompt("Would you like to use Config as Code?", "", true), | ||
testutil.NewSelectPrompt("Select where to store the Git credentials", "", []string{"Library", "Project"}, "Project"), | ||
testutil.NewInputPrompt("Git URL", "The URL of the Git repository to store configuration.", "https://github.com/blah.git"), | ||
testutil.NewInputPrompt("Git repository base path", "The path in the repository where Config As Code settings are stored. Default value is '.octopus/'.", "./octopus/project"), | ||
testutil.NewInputPrompt("Git branch", "The default branch to use. Default value is 'main'.", "main"), | ||
testutil.NewInputPrompt("Initial Git commit message", "The commit message used in initializing. Default value is 'Initial commit of deployment process'.", "init message"), | ||
testutil.NewInputPrompt("Git username", "The Git username.", "user1"), | ||
testutil.NewPasswordPrompt("Git password", "The Git password.", "password"), |
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.
🧑🍳 💋
|
||
func getGitBranch(opts *CreateOptions) string { | ||
if opts.GitBranch.Value == "" { | ||
return "main" |
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.
Lots of people have git branches where the default is 'master' so this might cause them problems?
I'm not sure how much effort we want to go to in solving this though.
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.
main
is the default in the UI and also the default when you create a new repo in GitHub, which has been the case for a couple of years (https://github.blog/changelog/2020-10-01-the-default-branch-for-newly-created-repositories-is-now-main/).
At Octopus, we also went through a process of swapping the default branch to main
where it was easy to do so. Server is still using master
, but most other repos are main
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.
For sure, but most companies are not nearly so forward-thinking as Octopus 😄 There's still a ton of 'master' out there. I can't say if this matters or not, but it'd be good to raise the question in a standup, if you haven't already, that's all. Cheers
[sc-18973]
This PR is currently blocked on PR OctopusDeploy/go-octopusdeploy#152 and the work on branch https://github.com/OctopusDeploy/go-octopusdeploy/tree/feat/git-credentials being merged
basic project create interaction:
basic project create with creating a project group inline:
Config-as-code project, with project stored credentials:
No-prompts: