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: configuration file of rules #91

Merged
merged 16 commits into from
Oct 14, 2024

Conversation

0xtekgrinder
Copy link
Contributor

fixes #82

This PR adds a few things:

  • A severity of issues and rules (info, warning and error)
  • Parsing of a configuration file to add new rules, update severity of rules or remove default rules
  • Appropriated documentation for configuration file and the new system when creating a rule
  • A flag to init an empty configuration file

This refactor a lot of code so I am really open to any criticism as I don't believe it is the best way to implement these features.
I also added a data field in the rules to be used later on.

Copy link
Contributor

@notJoon notJoon left a comment

Choose a reason for hiding this comment

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

Thank you for adding the feature you suggested recently. I believe this will become a very important functionality for linter.

However, I've left some comments that could be improved. Please check them when you have a chance. thanks!

internal/engine.go Outdated Show resolved Hide resolved
internal/engine.go Outdated Show resolved Hide resolved
internal/engine.go Show resolved Hide resolved
internal/engine.go Outdated Show resolved Hide resolved
@notJoon notJoon added the T-engine Type: related with engine (or internal) label Oct 13, 2024
@notJoon
Copy link
Contributor

notJoon commented Oct 14, 2024

@0xtekgrinder
The basic severity types are currently uniformly set to SeverityError, but it seems that re-adjustment will be necessary later. This is because most of them are more about style rather than actual errors, so I believe the most of linter's initial value should be at a warning or lower level.

Anyway, most of all, it looks good to me. Thanks for the big contribution. 👍

@notJoon notJoon merged commit 798dc04 into gnoverse:main Oct 14, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-engine Type: related with engine (or internal)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add severity level for issues
2 participants