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

Use golangci-lint #1044

Merged
merged 10 commits into from
Feb 8, 2021
Merged

Use golangci-lint #1044

merged 10 commits into from
Feb 8, 2021

Conversation

umarcor
Copy link
Contributor

@umarcor umarcor commented Feb 26, 2020

Close #952
Close #968

golangci-lint is added to Travis CI. Then, multiple linting issues are fixed: errors are handled, unused variables/functions are removed, unnecessary types are removed, frequently used strings are defined as constants, etc.

Ref #876

@umarcor umarcor changed the title Golangci lint golangci-lint Feb 26, 2020
.gitignore Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@jharshman jharshman changed the title golangci-lint use golangci-lint Feb 26, 2020
@jharshman jharshman changed the title use golangci-lint Use golangci-lint Feb 26, 2020
@umarcor umarcor force-pushed the golangci-lint branch 5 times, most recently from 3833ba1 to 22db848 Compare March 1, 2020 02:00
umarcor added a commit to umarcor/cobra that referenced this pull request Mar 1, 2020
@umarcor umarcor force-pushed the golangci-lint branch 2 times, most recently from 0744038 to a511ff5 Compare April 13, 2020 16:53
@umarcor
Copy link
Contributor Author

umarcor commented Apr 13, 2020

This PR is now rebased, and the issues introduced in the latest contributions to master are fixed. The failure of the labeler seems to be external.

umarcor added a commit to umarcor/cobra that referenced this pull request Apr 13, 2020
@umarcor umarcor mentioned this pull request Apr 13, 2020
@umarcor
Copy link
Contributor Author

umarcor commented May 11, 2020

I rebased and fixed the issues introduced in the latest contributions to master. Since the failing labeler was removed recently, all tests are green now.

umarcor added a commit to umarcor/cobra that referenced this pull request May 11, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request May 11, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request May 11, 2020
@umarcor
Copy link
Contributor Author

umarcor commented Jul 4, 2020

This PR was rebased again and some issues introduced in *completions* were fixed.

/cc @marckhouzam 39d3f42 (#1044)

umarcor added a commit to umarcor/cobra that referenced this pull request Jul 4, 2020
@umarcor umarcor force-pushed the golangci-lint branch 3 times, most recently from 1f6cd54 to 964c294 Compare July 11, 2020 12:32
umarcor added a commit to umarcor/cobra that referenced this pull request Jul 11, 2020
@umarcor
Copy link
Contributor Author

umarcor commented Feb 2, 2021

@jharshman, this is ready. Shall I move the last two commits to new PRs?

@jharshman
Copy link
Collaborator

@umarcor yes that would be great.

@umarcor
Copy link
Contributor Author

umarcor commented Feb 4, 2021

Done.

@jharshman
Copy link
Collaborator

@umarcor can you please squash your commits?

@umarcor
Copy link
Contributor Author

umarcor commented Feb 7, 2021

@jharshman I think that preserving the commits is meaningful in this case, because each of them fixes a different linter warning/error. They could have been multiple PRs. Anyway, GitHub's web UI should allow you to resolve this PR by Merge, Squash or Rebase, as you wish.

@jharshman jharshman merged commit 652c755 into spf13:master Feb 8, 2021
@umarcor umarcor deleted the golangci-lint branch February 8, 2021 05:51
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Nov 11, 2021
In version 1.1.2 of Cobra has been included a change[0] that changes the
behaviour of how custom usage functions are printed. The change prepends
"Error: " to the text. This is not desired since the usage text is not
an error text.

A workaround is to define a template string for the usage instead. The
template uses the templating language of Go[1]. See the default template
string in version 1.2.1[2].

This change has the positive benefit of getting more helpful usage
messages. E.g., for command 'toolbox enter' the usage text is no longer
"Run 'toolbox --help' for usage." but "Run 'toolbox enter --help' for usage.".

Upstream issue: spf13/cobra#1532

[0] spf13/cobra#1044
[1] https://pkg.go.dev/text/template
[2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Nov 16, 2021
In version 1.1.2 of Cobra has been included a change[0] that changes the
behaviour of how custom usage functions are printed. The change prepends
"Error: " to the text. This is not desired since the usage text is not
an error text.

Example of the wrong behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Error: Run 'toolbox --help' for usage.

Desired behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.

A workaround is to define a template string for the usage instead. The
template uses the templating language of Go[1]. See the default template
string in version 1.2.1[2].

Because the template is set only once, the executableBase needs to be
set before the template is applied. That required the move of
setUpGlobals() into init() of the cmd package. This is a better place
for the function call as init() is called earlier than Execute()[3].

Upstream issue: spf13/cobra#1532

[0] spf13/cobra#1044
[1] https://pkg.go.dev/text/template
[2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491
[3] https://golang.org/doc/effective_go#init
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Nov 16, 2021
In version 1.1.2 of Cobra has been included a change[0] that changes the
behaviour of how custom usage functions are printed. The change prepends
"Error: " to the text. This is not desired since the usage text is not
an error text.

Example of the wrong behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Error: Run 'toolbox --help' for usage.

Desired behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.

A workaround is to define a template string for the usage instead. The
template uses the templating language of Go[1]. See the default template
string in version 1.2.1[2].

Because the template is set only once, the executableBase needs to be
set before the template is applied. That required the move of
setUpGlobals() into init() of the cmd package. This is a better place
for the function call as init() is called earlier than Execute()[3].

Upstream issue: spf13/cobra#1532

[0] spf13/cobra#1044
[1] https://pkg.go.dev/text/template
[2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491
[3] https://golang.org/doc/effective_go#init

containers#917
HarryMichal added a commit to olivergs/toolbox that referenced this pull request Nov 21, 2021
In version 1.1.2 of Cobra has been included a change[0] that changes the
behaviour of how custom usage functions are printed. The change prepends
"Error: " to the text. This is not desired since the usage text is not
an error text.

Example of the wrong behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Error: Run 'toolbox --help' for usage.

Desired behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.

A workaround is to define a template string for the usage instead. The
template uses the templating language of Go[1]. See the default template
string in version 1.2.1[2].

Because the template is set only once, the executableBase needs to be
set before the template is applied. That required the move of
setUpGlobals() into init() of the cmd package. This is a better place
for the function call as init() is called earlier than Execute()[3].

Upstream issue: spf13/cobra#1532

[0] spf13/cobra#1044
[1] https://pkg.go.dev/text/template
[2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491
[3] https://golang.org/doc/effective_go#init

containers#917
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Dec 15, 2021
In version 1.1.2 of Cobra has been included a change[0] that changes the
behaviour of how custom usage functions are printed. The change prepends
"Error: " to the text. This is not desired since the usage text is not
an error text.

Example of the wrong behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Error: Run 'toolbox --help' for usage.

Desired behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.

A workaround is to define a template string for the usage instead. The
template uses the templating language of Go[1]. See the default template
string in version 1.2.1[2].

Because the template is set only once, the executableBase needs to be
set before the template is applied. That required the move of
setUpGlobals() into init() of the cmd package. This is a better place
for the function call as init() is called earlier than Execute()[3].

Upstream issue: spf13/cobra#1532

[0] spf13/cobra#1044
[1] https://pkg.go.dev/text/template
[2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491
[3] https://golang.org/doc/effective_go#init

containers#917
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Dec 16, 2021
In version 1.1.2 of Cobra has been included a change[0] that changes the
behaviour of how custom usage functions are printed. The change prepends
"Error: " to the text. This is not desired since the usage text is not
an error text.

Example of the wrong behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Error: Run 'toolbox --help' for usage.

Desired behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.

A workaround is to define a template string for the usage instead. The
template uses the templating language of Go[1]. See the default template
string in version 1.2.1[2].

Because the template is set only once, the executableBase needs to be
set before the template is applied. That required the move of
setUpGlobals() into init() of the cmd package. This is a better place
for the function call as init() is called earlier than Execute()[3].

Upstream issue: spf13/cobra#1532

[0] spf13/cobra#1044
[1] https://pkg.go.dev/text/template
[2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491
[3] https://golang.org/doc/effective_go#init

containers#917
debarshiray pushed a commit to HarryMichal/toolbox that referenced this pull request Dec 17, 2021
In version 1.1.2 of Cobra has been included a change[0] that changes
how custom usage functions are handled.

Example of the wrong behaviour:
$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.Error: Run 'toolbox --help' for usage.

Desired behaviour:
$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.

A workaround is to define a template string for the usage instead. The
template uses the templating language of Go[1]. See the default
template string in version 1.2.1[2].

Because the template is set only once, the executableBase needs to be
set before the template is applied. That required the move of
setUpGlobals() into init() of the cmd package. This is a better place
for the function call as init() is called earlier than Execute()[3].

Upstream issue: spf13/cobra#1532

[0] spf13/cobra#1044
[1] https://pkg.go.dev/text/template
[2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491
[3] https://golang.org/doc/effective_go#init

containers#917
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Dec 17, 2021
In version 1.1.2 of Cobra has been included a change[0] that changes
how custom usage functions are handled.

Example of the wrong behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.Error: Run 'toolbox --help' for usage.

Desired behaviour:

$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.

A workaround is to define a template string for the usage instead. The
template uses the templating language of Go[1]. See the default template
string in version 1.2.1[2].

Because the template is set only once, the executableBase needs to be
set before the template is applied. That required the move of
setUpGlobals() into init() of the cmd package. This is a better place
for the function call as init() is called earlier than Execute()[3].

Upstream issue: spf13/cobra#1532

[0] spf13/cobra#1044
[1] https://pkg.go.dev/text/template
[2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491
[3] https://golang.org/doc/effective_go#init

containers#917
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature A feature request for cobra; new or enhanced behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GitHub Actions
5 participants