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

build(deps): bump github.com/timonwong/logrlint from 0.1.0 to 0.4.0 #3140

Closed
wants to merge 1 commit into from

Conversation

timonwong
Copy link
Contributor

@timonwong timonwong commented Aug 26, 2022

timonwong/logrlint@v0.1.0...v0.3.0

Notable changes:

  • fix a fales positive for dotdotdot args
  • add support to klog
  • add config flags disableall, disable and enable

@timonwong timonwong changed the title feat: upgrade logrlint to v0.2.0; add config to logrlint feat: upgrade logrlint to v0.3.0; add config to logrlint Aug 26, 2022
@timonwong timonwong marked this pull request as ready for review August 26, 2022 04:29
@timonwong timonwong marked this pull request as draft August 26, 2022 07:05
@ldez
Copy link
Member

ldez commented Aug 26, 2022

FYI in the basic case, when a linter requires an update we let the dependabot handle that.

@ldez
Copy link
Member

ldez commented Aug 26, 2022

I would like to propose a better configuration:

linters-settings:
  logrlint:
    logr: true
    klog: true
    zap: true

@ldez ldez added the linter: update version Update version of linter label Aug 26, 2022
@ldez
Copy link
Member

ldez commented Aug 26, 2022

Why your PR is on draft?

@ldez ldez added the feedback required Requires additional feedback label Aug 26, 2022
@ldez ldez changed the title feat: upgrade logrlint to v0.3.0; add config to logrlint build(deps): bump github.com/timonwong/logrlint from 0.1.0 to 0.3.0 Aug 26, 2022
@timonwong
Copy link
Contributor Author

Why your PR is on draft?

@ldez It's still working in progress right now

@timonwong timonwong changed the title build(deps): bump github.com/timonwong/logrlint from 0.1.0 to 0.3.0 build(deps): bump github.com/timonwong/logrlint from 0.1.0 to 0.4.0 Aug 26, 2022
@ldez
Copy link
Member

ldez commented Aug 26, 2022

It's still working in progress right now

It's better to open a PR when the PR is ready because with a draft PR:

  • each commit will produce a notification
  • it's not possible to know the real state of the PR

When you open a draft PR, you must be clear on why and also explain what are the missing parts or the WIP.

@ldez
Copy link
Member

ldez commented Aug 26, 2022

Also, your linter name is based on logr, now you are trying to integrate klog, zap.

I think it can be better to create another linter with a better name that fits with the goal of your linter, and deprecate the previous linter.

@timonwong
Copy link
Contributor Author

Thanks, I'll close this PR for now. When it's ready, I'll open a new PR for review

@timonwong timonwong closed this Aug 26, 2022
@ldez ldez added declined and removed feedback required Requires additional feedback labels Aug 26, 2022
@timonwong timonwong deleted the upgrade-logrlint branch August 26, 2022 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
declined linter: update version Update version of linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants