Skip to content

Fix some basic tech debt #3

Merged
invidian merged 56 commits intomainfrom
invidian/base-fixes
Nov 2, 2021
Merged

Fix some basic tech debt #3
invidian merged 56 commits intomainfrom
invidian/base-fixes

Conversation

@invidian
Copy link
Copy Markdown
Member

@invidian invidian commented Oct 29, 2021

No description provided.

@invidian
Fix file formatting with gofumpt -w -s
5f09103 
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: initial commit
d8684ec 
Add basic configuration which currently does not report any issues, so
we can gradually re-enable remaining linters.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.github/workflows/go.yml: rename build job to test
51eb3a7 
As it only runs unit tests right now.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.github/workflows/go.yml: add lint job
649d5d4 
Running golangci-lint.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: fix error wrapping on error unmarshalling
bfea7cf 
To re-enable errorlint linter.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: re-enable errorlint linter
10f3403 
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: improve name of URL variable and struct field
157b784 
Also so we don't have a mix of exported struct fields and capitalized
variable names.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@tormath1
Copy link
Copy Markdown
Collaborator

tormath1 commented Oct 29, 2021

Good - thanks for this, I used goimports which basically runs a go fmt before committing files. Funny that it did not catch

var (
    toto = "toto"
)

to:

var toto = "toto"

invidian added 22 commits October 31, 2021 15:29
@invidian
pkg/client: rename variable ID -> id
8282967 
To avoid capitalized local variables and to enable gocritic linter.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable gocritic linter
a674d0d 
Now that all issues reported by it has been resolved.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: run all tests in parallel
8985817 
To enable paralleltest linter.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable paralleltest linter
d8c467d 
As it reports no issues anymore.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: add missing dots in comments
0bfa3bb 
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable godot linter
3bf6988 
So all comments ends with dots.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: improve formatting
01e4719 
To be able to re-enable wsl linter.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable wsl linter
24c0c4a 
Now that it does not report any issues.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
Consistently write FleetLock as "FleetLock" when possible
860d78c 
This is the formatting used in
https://coreos.github.io/zincati/development/fleetlock/protocol/
so I think this is the correct one which should be used when possible.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: fix ClientParams documentation styling
5360680 
So stylecheck linter does not complain.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
Add basic package comments
2206f52 
So stylecheck linter does not complain.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable stylecheck linter
b71ff23 
As all reported issues has been fixed.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: fix typo identifer -> identifier
2614160 
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable misspell linter
f968900 
As it does not find any more typos.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: fix too long line and duplicated error string in tests
4704413 
So lll linter can be enabled.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable lll linter
eb66470 
It does not report any issues anymore.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client/client_test.go: improve formatting
e87e1b2 
So nlreturn linter does not complain.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable nlreturn linter
465076d 
It does not report any issues anymore.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: rename ClientParams struct to just Params
988ef1c 
Otherwise revive linter complains with the following message:

exported: type name will be used as client.ClientParams by other
packages, and that stutters; consider calling this Params

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
cmd: add comment for exported variable Root
a79a6a0 
Even though it shouldn't really be exported as it makes it globally
writable, but this fixes revive linter.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable revive linter
3ed8192 
As it does not report any more issues.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: add nolint tag for client_params JSON field
e5fbfda 
As it is required by FleetLock protocol but it is not a standard case
for JSON format, so tagliatelle linter complains.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
invidian added 10 commits November 1, 2021 10:24
@invidian
.golangci.yml: enable govet linter
92a646f 
It does not report any issues anymore.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: add nolint comment on closing response body
d51ff0f 
So errcheck linter can be enabled.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: add error handling in tests
655b49d 
So errcheck linter can have extra checks enabled.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable errcheck linter
2c3dc97 
Now that it does not report any more issues.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable extra linter settings
298a8f8 
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: accept context for operations
c462242 
So they can be cancelled when take too much time.

For now fleetlockcli has no timeout defined by default.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable noctx linter
3b472be 
It does not report any issues anymore.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: get rid of magic numbers for HTTP response codes
b167c5e 
Replace it with more convinient ranges to satisfy gomnd linter.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: enable gomnd linter
507dc95 
It does not report any more issues.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.golangci.yml: update list of linters
3f2ab41 
So it matches auto-generated one.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian invidian force-pushed the invidian/base-fixes branch from 529a038 to e1b46cf Compare November 1, 2021 14:10
@invidian invidian mentioned this pull request Nov 1, 2021
Open
tormath1
tormath1 reviewed Nov 2, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@tormath1 tormath1 left a comment

Choose a reason for hiding this comment

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

This is great, some love to FleetLock :D Some comment, otherwise we're good to merge this. :)

@invidian
Copy link
Copy Markdown
Member Author

invidian commented Nov 2, 2021

Selection_489
image

invidian added 10 commits November 2, 2021 10:31
@invidian
Makefile: add targets for validating golangci-lint configuration
f170b88 
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.github/workflows/go.yml: check if list of linters is up to date
42eb797 
So new linters are either explicitly enabled or disabled while updating
the linter.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
Makefile: don't run build in parallel
86a59b3 
As with targets without associated source files it is usually not
desired to run things in parallel.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
Makefile: add help target
2ce5c53 
So each target has some helpful description.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
.github/workflows/go.yml: add spell checking job
2ff0ffc 
To catch more potential typos.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
pkg/client: move exported functions to the top of the file
421638c 
As if one looks at the source code, exported functions take precedence
in importance than unexported functions.

Also the order is that the New() is first, then the client methods, as
usually you need New() first to do other things.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
cmd: improve description of id flag
fc4b824 
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
main.go: exit with error exit code on execution error
60bf0cb 
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
main.go: don't print error message twice
a1262c8 
cobra already takes care of printing the execution error to stderr.

This also prevents printing the error message without newline at the
end.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian
cmd: add description for subcommands
0e1c5d3 
So when help is invoked, every available subcommand has some
description.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian invidian force-pushed the invidian/base-fixes branch from 56f44f5 to 0e1c5d3 Compare November 2, 2021 09:32
@invidian invidian requested a review from tormath1 November 2, 2021 09:33
tormath1
tormath1 reviewed Nov 2, 2021
View reviewed changes
tormath1
tormath1 approved these changes Nov 2, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@tormath1 tormath1 left a comment

Choose a reason for hiding this comment

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

One last comment, let's merge it then :)
Thanks for this :D

@invidian invidian merged commit 8937d30 into main Nov 2, 2021
@invidian invidian deleted the invidian/base-fixes branch November 2, 2021 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tormath1 tormath1 tormath1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@invidian @tormath1