Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
refactor: Make config internal to CLI #2310
refactor: Make config internal to CLI #2310
Changes from 7 commits
daf61f6
da779cd
42296e4
19359da
9739882
6bd738a
4ee8d50
de99a36
19e428b
7b049a8
9dfb1b6
25c427b
225f02e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 27 in cli/client.go
Codecov / codecov/patch
cli/client.go#L26-L27
Check warning on line 41 in cli/collection.go
Codecov / codecov/patch
cli/collection.go#L40-L41
Check warning on line 97 in cli/config.go
Codecov / codecov/patch
cli/config.go#L96-L97
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.
todo: Can you please document the reasons for the
nolint
stuff with code-comments please (in all locations).Check warning on line 114 in cli/config.go
Codecov / codecov/patch
cli/config.go#L113-L114
Check warning on line 121 in cli/config.go
Codecov / codecov/patch
cli/config.go#L120-L121
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.
praise: Thank you for comments like this, it helps a lot when the code itself is non-obvious.
Check warning on line 138 in cli/config.go
Codecov / codecov/patch
cli/config.go#L137-L138
Check warning on line 149 in cli/config.go
Codecov / codecov/patch
cli/config.go#L148-L149
Check warning on line 157 in cli/config.go
Codecov / codecov/patch
cli/config.go#L152-L157
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.
suggestion: It is annoying that the linter flags this, but I think that using a const (even though that is also bad here) is slightly less bad that a
nolint
.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 moved all of them to
const
for now. They should be removed when the new logging package is ready.Check warning on line 163 in cli/config.go
Codecov / codecov/patch
cli/config.go#L162-L163
Check warning on line 167 in cli/config.go
Codecov / codecov/patch
cli/config.go#L166-L167