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

minphony false positives #87

Open
mcandre opened this issue Mar 21, 2023 · 3 comments
Open

minphony false positives #87

mcandre opened this issue Mar 21, 2023 · 3 comments

Comments

@mcandre
Copy link

mcandre commented Mar 21, 2023

Please don't enforce a PHONY declaration for tasks that are not declared in our Makefiles.

That is, if clean isn't even implemented, then it should not be marked PHONY. Same goes for test and all.

One could argue that all, test, and clean are idiomatic tasks that every Makefile should implement. While I personally useful default tasks, TDD, and practical cleanup tasks, even so, I think that would be an overbearing rule to inflict on users.

Hmm, looks like the minphony rule can't even be disabled by checkmake.ini? It is to cry.

@mcandre mcandre changed the title Relax default behavior for minphony minphony false positives Mar 24, 2023
@MajorDallas
Copy link

Falls under the title, but different from OP: I do have a required target and minphony is telling me that I do not.

Knowing nothing about how the parser works, the only thing I can think of that could be confusing it is that the phony target has a real target for a prerequisite.

.PHONY : reqs container_rebuild container_update test test-now print
# ...
test : tests/.last_test
tests/.last_test : $(PY_SRC_FILES)
    cd tests; poetry run pytest . && touch .last_test
checkmake Makefile
    RULE              DESCRIPTION             FILE NAME   LINE NUMBER

  minphony   Missing required phony target    Makefile    0
             "all"
  minphony   Missing required phony target    Makefile    0
             "clean"
  minphony   Missing required phony target    Makefile    0
             "test"

@mcandre
Copy link
Author

mcandre commented Oct 22, 2023

@cooljeanius
Copy link

cooljeanius commented Nov 15, 2023

PR #88 was meant to close issue #86, but I think it would also at least somewhat address this one, as well.
See also issue #72 as well.
Edit: and issue #15 too.

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

No branches or pull requests

3 participants