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.

Sign up for GitHub

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

Jump to bottom

appdev: update flag documentation, scope timeout flag to actual build #1250

Merged
merged 2 commits into from
Sep 26, 2022
Merged

appdev: update flag documentation, scope timeout flag to actual build #1250

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e83b0bf
update flag documentation, scope timeout flag to actual build
kamaln7 Sep 23, 2022
dad2e40
Merge branch 'main' into feature/appdev-flag-updates
andrewsomething Sep 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 29 additions & 22 deletions commands/apps_dev.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,14 @@ func AppsDev() *Command {
Command: &cobra.Command{
Use: "dev",
Aliases: []string{},
Short: "[BETA] Display commands for working with app platform local development.",
Long: `[BETA] Display commands for working with app platform local development.`,
Hidden: true,
Short: "[BETA] Display commands for working with App Platform local development.",
Long: heredoc.Docf(`
[BETA] Display commands for working with App Platform local development.
To get started, run %s.`,
"`doctl app dev build`",
),
Hidden: true,
},
}

Expand All @@ -58,10 +63,14 @@ func AppsDev() *Command {
RunAppsDevBuild,
"build [component name]",
"Build an app component",
heredoc.Doc(`
heredoc.Docf(`
[BETA] Build an app component locally.
The component name must be specified as an argument if running non-interactively.`,
The component name is optional unless running non-interactively.
All command line flags as optional. You may specify flags to be applied to the current build
or use the command %s to permanently configure default values.`,
"`doctl app dev config`",
),
Writer,
aliasOpt("b"),
Expand All @@ -71,7 +80,7 @@ func AppsDev() *Command {
AddStringFlag(
build, doctl.ArgAppSpec,
"", "",
`Path to an app spec in JSON or YAML format.`,
`An optional path to an app spec in JSON or YAML format. Default: .do/app.yaml.`,
)

AddStringFlag(
Expand All @@ -83,39 +92,40 @@ func AppsDev() *Command {
AddStringFlag(
build, doctl.ArgEnvFile,
"", "",
"Additional environment variables to inject into the build.",
"An optional path to a .env file with overrides for values of app spec environment variables.",
)

AddBoolFlag(
build, doctl.ArgNoCache,
"", false,
"Whether or not to omit the cache for the build.",
"Set to disable build caching.",
)

AddStringFlag(
build, doctl.ArgBuildCommand,
"", "",
"Optional build command override for local development.",
"An optional build command override for local development.",
)

AddDurationFlag(
build, doctl.ArgTimeout,
"", 0,
"An optional timeout duration for the build",
`An optional timeout duration for the build. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". Example: 15m30s`,
Copy link
Contributor

Choose a reason for hiding this comment

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

You could probably drop the small time values, they seem impractical, to make this more concise and reduce clutter.

Suggested change
`An optional timeout duration for the build. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". Example: 15m30s`,
`An optional timeout duration for the build. Valid time units are "s", "m", "h". Example: 15m30s`,

)

AddStringFlag(
build, doctl.ArgRegistry,
"", os.Getenv("APP_DEV_REGISTRY"),
"Registry name to build use for the component build.",
"An optional registry name to tag built container images with.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a suggestion in case ending a sentence with a preposition concerns people. :-)

Suggested change
"An optional registry name to tag built container images with.",
"An optional registry name used to tag built container images.",

)

return cmd
}

// RunAppsDevBuild builds an app component locally.
func RunAppsDevBuild(c *CmdConfig) error {
ctx := context.Background()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

ws, err := appDevWorkspace(c)
if err != nil {
Expand All @@ -139,12 +149,6 @@ func RunAppsDevBuild(c *CmdConfig) error {

template.Print("{{muted pointerRight}} current app dev workspace: {{muted .}}{{nl}}", ws.Context())

if ws.Config.Timeout > 0 {
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, ws.Config.Timeout)
defer cancel()
}

if ws.Config.AppSpec == nil {
err := appsDevBuildSpecRequired(ws, c.Apps())
if err != nil {
Expand Down Expand Up @@ -287,7 +291,7 @@ func RunAppsDevBuild(c *CmdConfig) error {
}

if builder.IsCNBBuild(componentSpec) && ws.Config.CNBBuilderImage != "" {
template.Render(text.Warning, `{{pointerRight}} using custom builder image {{highlight .}}{{nl}}`, ws.Config.CNBBuilderImage)
template.Render(text.Warning, `{{checkmark}} using custom builder image {{highlight .}}{{nl}}`, ws.Config.CNBBuilderImage)
}

// if Interactive {
Expand All @@ -303,7 +307,12 @@ func RunAppsDevBuild(c *CmdConfig) error {
// return fmt.Errorf("canceled")
// }
// }

if ws.Config.Timeout > 0 {
template.Render(text.Warning, `{{checkmark}} restricting maximum build duration to {{highlight (duration .)}}{{nl}}`, ws.Config.Timeout)
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, ws.Config.Timeout)
defer cancel()
}
buildingComponentLine := template.String(
`building {{lower (snakeToTitle .componentSpec.GetType)}} {{highlight .componentSpec.GetName}} {{muted (print "(" .appName ")")}}`,
map[string]any{
Expand All @@ -320,8 +329,6 @@ func RunAppsDevBuild(c *CmdConfig) error {
// userCanceled indicates whether the context was canceled by user request
userCanceled bool
)
ctx, cancel := context.WithCancel(ctx)
defer cancel()
if Interactive {
logPager, err := pager.New(
pager.WithTitle(buildingComponentLine),
Expand Down