How to use cycle detection in packs, a Rust-based packwerk clone?

Tools RubyAtScale
1

This message was imported from the Ruby/Rails Modularity Slack server. Find more info in the import thread.

For anyone following around/playing with packs (packwerk clone built in Rust), we now have cycle detection! Packs behaves a bit different than packwerk here:

  1. Instead of validate, packs has 1 command instead of 2, so cycle detection runs with packs check
  2. The output shows groups of strongly connected components (whereas packwerk shows a sampling of specific paths through cycles)
2

Message originally sent by slack user U717FXJ8HWK

I wrote a comment on the PR about merging the two commands: https://github.com/alexevanczuk/packs/pull/64#pullrequestreview-1521408688

Having the same command do both breaks expectations for a drop-in replacement, e.g., you can no longer put packs check into a pre-commit hook if you have cyclic dependencies that you currently ignore.

I think keeping the CLI as close as possible to packwerk would help with adoption.

3

Oh right – I forgot some users of packwerk don’t use cycle detection.

Having separate commands for check and validate would be fine, but I’m trying to see if we can have our cake and eat it too here. I wonder if we can have packwerk.yml support a flag to do cycle detection within check (or some flag to disable cycle detection, since the default mode in packwerk is that it’s on).

What do you think?

4

Message originally sent by slack user U717FXJ8HWK

With a new configuration option we would diverge from Packwerk even more, I’d keep everything as close to Packwerk as possible. :thinking_face:

5

True — I was more thinking that the new option could be for packs users only (e.g. validate_on_check). But I see your point – it’s nice to keep things as close to possible with the vanilla configuration for now.

6

https://github.com/alexevanczuk/packs/pull/74

7

Thank you for the feedback :pray:

8

Message originally sent by slack user U717FXJ8HWK

I have more. :sweat_smile: There seems to be a regression caused by https://github.com/alexevanczuk/packs/pull/70 where check doesn’t report any violations even though an older version and bin/packwerk check do

9

Huh – if you packs delete-cache does it still happen?

10

Message originally sent by slack user U717FXJ8HWK

Seems to work fine after deleting cache

11

Message originally sent by slack user U717FXJ8HWK

scenario 1:
• packs delete-cache
• packs-old check - works
• packs check - no violations
scenario 2:
• packs delete-cache
• bin/packwerk check - works
• packs check - works :thinking_face:

12

Message originally sent by slack user U717FXJ8HWK

While we’re at it: https://github.com/alexevanczuk/packs/pull/75 :smile:

13

Sounds like that issue is fixed now. I think it might have been a bad cache? Let me know if you see it again.

14

PR looks good!

15

Had some questions/comments