-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
add tagalign linter #3709
add tagalign linter #3709
Conversation
Hey, thank you for opening your first Pull Request ! |
In order for a pull request adding a linter to be reviewed, the linter and the PR must follow some requirements. Pull Request Description
Linter
The Linter Tests Inside Golangci-lint
|
hi team, ready for review. |
fix type Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
@alexandear Really thanks for your suggestion. A new test case with auto-sort disabled have been added. Try: T=tagalign_no_sort.go make test_linters |
The modifications required by the checklist are not done. @alexandear It's better to wait before reviewing that the user part of the checklist is done. |
If there are anything I need to do, just let me know. : ) |
Follow the checklist except for boxes with the message "(the team will help to verify that)" |
IMO the alignment of the tags is less interesting than the sort of the tags, maybe it can be a good idea to add an option to enable/disable the alignment. But this improvement is not a blocker to merging this PR. |
Nice suggestion. I have added a |
I change a bit the default, the new default: Otherwise, I modified the tests to have simpler test cases, and I can see that some options don't work as expected: if |
Found a bug here. Should work now. |
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.
Thanks for your contribution 👍
tagalign is used to align and sort tags in golang struct.
eg:
can be aligned to
It can also sort tags with user's prefered order.