rules: add uniquetargets rule to detect repeated Makefile targets#171
Conversation
obnoxxx
left a comment
There was a problem hiding this comment.
This is a nice rule addition.
The ci is failing on the pre-commit check
and it seems to be due to #172.
So this should be good if we can fix that issue first.
Additionally, my request is to document the new rule in the manpage in this PR.
obnoxxx
left a comment
There was a problem hiding this comment.
One more suggestion/request:
Should we rename the new rule?
Exesting rules have names that (positively) specify something that a Makefile has to fulfill to pass
This new rule's name (negatively) specifies something that a Makefile must not fulfill to pass.
how about a name like uniqueargets? I think it would be more consistent with the existing rule names.
1907920 to
d22a009
Compare
d22a009 to
2ec7fbe
Compare
2ec7fbe to
5213ecd
Compare
5213ecd to
d875b4e
Compare
This new rule warns when the same target is defined multiple times, which can cause recipe overrides in GNU Make or unintended merges in BSD Make. The rule supports an optional `ignore` configuration key to skip specific targets. Signed-off-by: Mikel Olasagasti Uranga <[email protected]>
d875b4e to
3395ccb
Compare
|
I removed fixtures from the PR, as I used them during the development but are not required by |
What would be missing to document the rule somewhere, but I think that's missing for all rules. |
|
@mikelolasagasti wrote:
yes. I think we should add rule descriptions to the manpage: Maybe even add a new section for rules to the manpage. |
|
@mikelolasagasti wrote:
Sounds good.
Agreed. The go test file is meant to hold unit tests, not end-to-end CLI tests. |
|
|
||
| // Validate let's you validate a passed in Makefile with the provided config | ||
| func Validate(makefile parser.Makefile, cfg *config.Config) (ret rules.RuleViolationList) { | ||
|
|
There was a problem hiding this comment.
I am not quite sure why the patch removes this empty line
as it does not seem to be related to what the PR does.
I guess it was done by your editor/IDE automatically
and added to the commit by accident.
I'll therefore ignore it as it does not harm either.
This new rule warns when the same target is defined multiple times, which can cause recipe overrides in GNU Make or unintended merges in BSD Make. The rule supports an optional
ignoreconfiguration key to skip specific targets.