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

Clarify the description of "namespace" in the UI and CLI #1023

Merged
merged 8 commits into from
Nov 8, 2021
Merged

Clarify the description of "namespace" in the UI and CLI #1023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cedaad3
changed namespace text in ApplicationAdd, the persistent flag in gito…
joshri Nov 3, 2021
e3b4c4e
changed namespace flag text and edited install and uninstall tests to…
joshri Nov 4, 2021
8e1ae5a
fixes makefile and changes namespace flag text in all locations
joshri Nov 4, 2021
e25c55b
remove duplicate "default wego-system"
joshri Nov 5, 2021
5e930b7
remove two periods that are messing with the tests
joshri Nov 5, 2021
cfbcf3c
removed extra parens that were messing with tests. I think I have don…
joshri Nov 5, 2021
ac94bea
if this period placement is not correct, expect real human tears to f…
joshri Nov 5, 2021
88efacb
Justin taught me how to do tests, and now things look great
joshri Nov 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ merged.lcov:
# Thanks to https://www.thapaliya.com/en/writings/well-documented-makefiles/
help: ## Display this help.
ifeq ($(OS),Windows_NT)
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n <target>\n"} /^[a-zA-Z_-]+:.*?##/ { printf " %-40s %s\n", $$1, $$2 } /^##@/ { printf "\n%s\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make <target>\n"} /^[a-zA-Z_-]+:.*?##/ { printf " %-40s %s\n", $$1, $$2 } /^##@/ { printf "\n%s\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
else
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n \033[36m<target>\033[0m\n"} /^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-40s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-40s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
endif
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Available Commands:

Flags:
-h, --help Help for gitops
--namespace string Weave GitOps runtime namespace (default "wego-system")
--namespace string The namespace scope for this operation (default "wego-system").
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a change I did in a branch. Trying to be super specific about what this flag does.

Suggested change
--namespace string The namespace scope for this operation (default "wego-system").
--namespace string The Kubernetes namespace in which Weave GitOps objects will be stored (default "wego-system").

Copy link
Contributor

Choose a reason for hiding this comment

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

@jpellizzari specific can be tricky... doing a get/status or remove isn't a "will be", it's "the objects are here already"

Copy link
Contributor

Choose a reason for hiding this comment

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

@sympatheticmoose Not sure what you mean. We are open to suggestions on the text itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

I gave my suggestion already in the issue ;) I'm saying "will be stored" implies that this is always an action where I am about to store objects which don't yet exist into the given namespace. Which isn't the case for read or remove operations.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK I missed that. @joshri ^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it 🌞

-v, --verbose Enable verbose output

Use "gitops [command] --help" for more information about a command.
Expand Down
2 changes: 1 addition & 1 deletion cmd/gitops/root/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func RootCmd(client *resty.Client) *cobra.Command {
}

rootCmd.PersistentFlags().BoolVarP(&options.verbose, "verbose", "v", false, "Enable verbose output")
rootCmd.PersistentFlags().String("namespace", wego.DefaultNamespace, "Weave GitOps runtime namespace")
rootCmd.PersistentFlags().String("namespace", wego.DefaultNamespace, "The namespace scope for this operation.")
rootCmd.PersistentFlags().StringVarP(&options.endpoint, "endpoint", "e", os.Getenv("WEAVE_GITOPS_ENTERPRISE_API_URL"), "The Weave GitOps Enterprise HTTP API endpoint")
rootCmd.PersistentFlags().BoolVar(&options.overrideInCluster, "override-in-cluster", false, "override running in cluster check")
cobra.CheckErr(rootCmd.PersistentFlags().MarkHidden("override-in-cluster"))
Expand Down
4 changes: 2 additions & 2 deletions test/acceptance/test/install_tests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var _ = Describe("Weave GitOps Install Tests", func() {

By("Then I should see gitops help text displayed for 'install' command", func() {
Eventually(string(sessionOutput.Wait().Out.Contents())).Should(MatchRegexp(
fmt.Sprintf(`The install command deploys GitOps in the specified namespace.\nIf a previous version is installed, then an in-place upgrade will be performed.\n*Usage:\n\s*gitops install \[flags]\n*Examples:\n\s*# Install GitOps in the %s namespace\n\s*gitops install\n*Flags:\n\s*--dry-run\s*Outputs all the manifests that would be installed\n\s*-h, --help\s*help for install\n*Global Flags:\n\s*-e, --endpoint string\s*The Weave GitOps Enterprise HTTP API endpoint\n\s*--namespace string\s*Weave GitOps runtime namespace \(default "%s"\)\n\s*-v, --verbose\s*Enable verbose output`, wego.DefaultNamespace, wego.DefaultNamespace)))
fmt.Sprintf(`The install command deploys GitOps in the specified namespace.\nIf a previous version is installed, then an in-place upgrade will be performed.\n*Usage:\n\s*gitops install \[flags]\n*Examples:\n\s*# Install GitOps in the %s namespace\n\s*gitops install\n*Flags:\n\s*--dry-run\s*Outputs all the manifests that would be installed\n\s*-h, --help\s*help for install\n*Global Flags:\n\s*-e, --endpoint string\s*The Weave GitOps Enterprise HTTP API endpoint\n\s*--namespace string\s*The namespace scope for this operation. \(default "%s"\)\n\s*-v, --verbose\s*Enable verbose output`, wego.DefaultNamespace, wego.DefaultNamespace)))
})
})

Expand All @@ -48,7 +48,7 @@ var _ = Describe("Weave GitOps Install Tests", func() {

By("Then I should see gitops help text displayed for 'uninstall' command", func() {
Eventually(string(sessionOutput.Wait().Out.Contents())).Should(MatchRegexp(
fmt.Sprintf(`The uninstall command removes GitOps components from the cluster.\n*Usage:\n\s*gitops uninstall \[flags]\n*Examples:\n\s*# Uninstall GitOps from the %s namespace\n\s*gitops uninstall\n*Flags:\n\s*--dry-run\s*Outputs all the manifests that would be uninstalled\n\s*-h, --help\s*help for uninstall\n*Global Flags:\n\s*-e, --endpoint string\s*The Weave GitOps Enterprise HTTP API endpoint\n\s*--namespace string\s*Weave GitOps runtime namespace \(default "%s"\)\n\s*-v, --verbose\s*Enable verbose output`, wego.DefaultNamespace, wego.DefaultNamespace)))
fmt.Sprintf(`The uninstall command removes GitOps components from the cluster.\n*Usage:\n\s*gitops uninstall \[flags]\n*Examples:\n\s*# Uninstall GitOps from the %s namespace\n\s*gitops uninstall\n*Flags:\n\s*--dry-run\s*Outputs all the manifests that would be uninstalled\n\s*-h, --help\s*help for uninstall\n*Global Flags:\n\s*-e, --endpoint string\s*The Weave GitOps Enterprise HTTP API endpoint\n\s*--namespace string\s*The namespace scope for this operation. \(default "%s"\)\n\s*-v, --verbose\s*Enable verbose output`, wego.DefaultNamespace, wego.DefaultNamespace)))
})
})

Expand Down
2 changes: 1 addition & 1 deletion ui/pages/ApplicationAdd.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ function AddApplication({ className }: Props) {
value={formState.namespace}
/>
<FormHelperText>
The the target namespace for the application
The namespace where GitOps automation objects will be stored.
</FormHelperText>
</FormElement>
<FormElement>
Expand Down