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

update doc #1203

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

update doc #1203

wants to merge 3 commits into from

Conversation

mfederowicz
Copy link
Contributor

@mfederowicz mfederowicz commented Jan 22, 2025

related with #1194

  • 1. done
  • 2. done
  • 3. @alexandear did you have any extended examples to this section
  • 4. done - because we have: See our developer guide for instructions on building the project.

DEVELOPING.md Outdated Show resolved Hide resolved
DEVELOPING.md Outdated Show resolved Hide resolved
DEVELOPING.md Outdated Show resolved Hide resolved
DEVELOPING.md Outdated
@@ -31,7 +23,7 @@ The command will produce the `revive` binary in the root of the project.

If you want to develop a new rule, follow as an example the already existing rules in the [rule package](https://github.com/mgechev/revive/tree/master/rule).

All rules should implement the following interface:
Each rule needs to implement the `lint.ConfigurableRule` interface. All rules with a configuration must implement that interface:
Copy link
Collaborator

Choose a reason for hiding this comment

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

These sentences are not clear to me.
AFAIK rules must not necessary implement Configurable (thus Each rule needs to implement Configurable is not true)

I understand "that" in "All rules with a configuration must implement that interface" as the Configurable interface but following the sentence there is the code of Rule interface (no that of Configurable).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok now should be better :)

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