ApplicationValidator check

[oskarpearson]

Hi all

What do people think of changing ApplicationValidator so that there's a public API for retrieving the actual results, rather than a text-version-of-the-results-joined-with-slash-ns?

Context:

Our current project does contain package cycles. While this isn't ideal, we're trying to work towards something without cycles over time.

Previously, I had a pretty hacky spec in our project which checked that our packwerk config was valid excluding the acyclic references check. It did this by instantiating ApplicationValidator and excluding that check.

I see that ApplicationValidator has been marked as a private constant in recent versions of Packwerk in change e1da318 Additionally, the structure of validators has changed - so Packwerk::Validator.all returns a list of validators, and this is dynamically used. Nice.

However, ideally I'd like to be able to get the results of validate in a non-text-form and process them for compliance in each execution of CI. Otherwise I'm forced to use the Packwerk CLI and then try process the text results with string matching.

Some ideas I can see working:

Option 1:

• Make Packwerk::ApplicationValidator accessible again
• Change Validators like Packwerk::Validators::DependencyValidator so that they don't worry about formatting (ie: remove calls to merge_results from them, and instead return a consistent data structure)
• Change Packwerk::ApplicationValidator#call to return just results (ie not merge_results(results, separator: "\n❓ ")) in call)
• Change the CLI to use some sort of "formatter" to handle merging the results for display in the CLI

The advantage of this is that it separates out the 'view' from the validation. This means that I could write more intelligent checkers ("We've noticed you've increased the number of cycles by 1 in this PR", and "Yay, you've reduced the number of cycles by 3 in this PR").

Option 2:

• Make Packwerk::ApplicationValidator accessible again.
• Add config options that allow people to selectively disable/enable some checkers.

This way I can at least check everything other than app cycles in CI.

Option 3:

Input as to other options welcome!

Thanks for your time!

Oskar

[exterm]

Our current project does contain package cycles.

There are two things to distinguish here:

• actual cyclical dependencies
• specified cyclical dependencies

Actual dependencies can be cyclical, packwerk has no problem with that.
When we decided to have packwerk reject cyclical specified dependencies, this was the reasoning:

Specified dependencies express a desired state. The desired state should never contain cycles. Packwerk enables teams to move from current state towards desired state via the package todo files which allow recording the difference between current and desired states. They are intended to be worked off (reduced) over time.

Allowing cyclical dependencies in specified dependencies makes the difference between current and desired state less obvious, so I don't think it makes sense in packwerk and I can't see how manual workarounds would be very useful either.

Maybe the docs don't explain this very well.

Does this make sense to you? Could you specify a desired dependency graph in your package.yml files and use package todo files to capture the difference between that desired state and the current state?

[oskarpearson]

Hey @exterm

Thanks for your answer here - makes a lot of sense. I have been considering the package.yml being reflective of the "actual" state rather than desired state.

Part of this is probably because of the previous mix in package_deprecations.yml file purpose (privacy and dependency), but the above answer has been enlightening. Ta!