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

packs update adds todos for strict violations #166

Closed
perryqh opened this issue Mar 8, 2024 · 2 comments
Closed

packs update adds todos for strict violations #166

perryqh opened this issue Mar 8, 2024 · 2 comments

Comments

@perryqh
Copy link
Contributor

perryqh commented Mar 8, 2024

"Bug"

Running pks update will add meaningless "todo" entries for strict violations. pks check behaves as expected in that it detects/reports strict errors regardless of what is in todo files.

Proposed Change:

  • pks update to no longer update todos with strict violations
  • pks update outputs skipped strict violations so that the user can expect a pks check error.
  • pks check remains unchanged

How?

Add a new strict_mode bool to the ViolationIdentifier struct. It will be populated in the "checkers" and be used to ignore violations when writing out todos and detecting strict_mode violations.

Instead of a strict_mode bool we could also consider an enum. I'm not sure if the complexity is warranted at this time.

Packwerk

As an aside, packwerk will not update todo files with strict violations and will also not report packwerk check strict violations if they have a corresponding todo entry. Supporting pks check ignoring strict todo violations is outside the scope of this issue.

Feedback

Thoughts? Better approaches?

@alexevanczuk
Copy link
Owner

Bool on the violation identifier sounds great! As you mentioned we can easily refactor to an enum later if needed.

Thank you!!

@perryqh
Copy link
Contributor Author

perryqh commented Apr 2, 2024

@perryqh perryqh closed this as completed Apr 2, 2024
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

2 participants