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

fix: handle LEFTHOOK_QUIET when there is no skip_output in config #630

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

prog-supdex
Copy link
Contributor

Closes #623

⚡ Summary

Handles situations when only LEFTHOOK_QUIET exists, without skip_output in the lefthook.yml file

☑️ Checklist

  • Check locally
  • Add tests
  • Add documentation

Copy link
Member

@mrexox mrexox left a comment

Choose a reason for hiding this comment

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

This fix solves the issue! Thank you! I only have a nitpick about the tests

Comment on lines 110 to 125
t.Run("ApplySettings with non-empty tags and nil skipOutput", func(t *testing.T) {
var settings SkipSettings
(&settings).ApplySettings("meta,success", nil)

if !settings.SkipMeta() {
t.Errorf("expected SkipMeta to be true")
}

if !settings.SkipSuccess() {
t.Errorf("expected SkipSuccess to be true")
}

if settings.SkipFailure() {
t.Errorf("expected SkipFailure to be false")
}
})
Copy link
Member

Choose a reason for hiding this comment

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

What if we extend the previous test to include tags variable? I think we can add a testcase with tags: "meta,success" and settings: nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

@prog-supdex prog-supdex merged commit ed10e37 into evilmartians:master Feb 15, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LEFTHOOK_QUIET enviroment variable doesn't work
2 participants