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

feat: implement --forbidden-keys #40

Merged
merged 6 commits into from
May 9, 2024
Merged

feat: implement --forbidden-keys #40

merged 6 commits into from
May 9, 2024

Conversation

tigrato
Copy link
Contributor

@tigrato tigrato commented May 8, 2024

This PR implements a linter rule to deny usages of certain keys as attributes to enforce that developers don't override reserved keys managed by custom handlers.

@tigrato tigrato changed the title feat: implement --forbidden-key feat: implement --reserved-key May 8, 2024
@tmzane tmzane self-requested a review May 9, 2024 12:03
@tmzane
Copy link
Member

tmzane commented May 9, 2024

Hello, thank you for the PR, I like the idea!

I noticed that you first named it --forbidden-key, which I actually like better, because there might be other reasons why a log key is forbidden (e.g., a styleguide). In general, I think sloglint does not need to know the semantics behind the keys, it just needs to forbid them.

@tmzane tmzane requested a review from mattdowdell May 9, 2024 12:09
@tigrato tigrato changed the title feat: implement --reserved-key feat: implement --forbidden-key May 9, 2024
@tigrato
Copy link
Contributor Author

tigrato commented May 9, 2024

@tmzane thanks.

I reverted the feature to its original name forbbiden-key.

This PR implements a linter rule to deny usages of certain keys as
attributes to enforce that developers don't override reserved keys managed by
custom handlers.

Signed-off-by: Tiago Silva <[email protected]>
sloglint.go Outdated Show resolved Hide resolved
sloglint.go Outdated Show resolved Hide resolved
sloglint.go Outdated Show resolved Hide resolved
sloglint_test.go Outdated Show resolved Hide resolved
sloglint_test.go Outdated Show resolved Hide resolved
@tigrato tigrato requested a review from tmzane May 9, 2024 14:08
Copy link
Collaborator

@mattdowdell mattdowdell left a comment

Choose a reason for hiding this comment

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

LGTM on the code side, just a few minor comments about the docs.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@tmzane
Copy link
Member

tmzane commented May 9, 2024

I just remembered that strconv.Unquote exists. And it is used in the standard library exactly for this case!

@tmzane tmzane changed the title feat: implement --forbidden-key feat: implement --forbidden-keys May 9, 2024
@tmzane tmzane merged commit 431c6b7 into go-simpler:main May 9, 2024
8 checks passed
@tmzane
Copy link
Member

tmzane commented May 9, 2024

@tigrato thanks, this is a great feature to have!

@mattdowdell as always, thanks for the review!

@tmzane
Copy link
Member

tmzane commented May 9, 2024

Oh, and we should bump Go version in go.mod, because the slices package only exists since 1.21.

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.

3 participants