Shopify/packwerk

[Feature Request] Allow configuration of `package_todo.yml` violation destinations

jakebrady5 opened this issue · 4 comments

Proposal

Allowing configuration for whether a violation gets reported in either the offending package's package_todo.yml or that of the offended package will allow users better control over how they track and assign ownership of packwerk violations.

Motivation

We are working to incrementally adopt packwerk in the GitHub monolith. Particularly with the privacy checker, we have had multiple teams request that violations against private constants be tracked in the todo file for the package that owns the constant, rather than distributed across each offending package.

Adding new privacy violations to the offended package's todo file allows the codeowners feature to automatically tag the impacted team to review the PR.

Adopting packwerk is very much an incremental process for our application, with the journey to strict enforcement levels containing multiple stages. Being able to configure violation destinations helps us to match packwerk reporting more closely to our organizational expectations for how violations are owned and worked through.

Context

A version of this feature had a prior PR. Some time has passed since that initial discussion, so I would be curious to hear your updated opinions.

The PR solves our request essentially as-is. I could also see an argument for making the destination more user configurable if there is concern about the broader community agreeing with our destination preferences.

At gusto, several of our teams have also requested this feature. When I'm trying to figure out what privacy violation to tackle next, I'm often using RubyMine tricks to get a coherent view of the privacy violations for my pack.

Thank you for the issue, but my opinion continue the same #261 (comment).

I'd totally accept a PR to change the database to use a different format that allow us to write any queries we want, so team can answer the questions they have about their package. But, the place were the violation are recorded in the repository should stay the same.

Also, this is mostly for the privacy checks, that aren't part of Packwerk anymore, and we (Shopify) discourage the usage of the current implementation of privacy checks.

Thanks, @rafaelfranca. I can appreciate the simplicity from the library’s perspective of keeping the rule consistent for violation reporting location. I can also understand that adding this feature is all cost and no benefit for your use case if you’re not interested in the privacy checker.

Our primary motivation for violation destination configuration is to enable ownership, responsibility, and notifications for violations to happen as automatically as possible. We (unsurprisingly) lean into GitHub functionality as a first measure, and like the diff-ability and codeowners integrations that text files provide. While we could accomplish our goals with actions or scripts external to Packwerk, the issue proposal removes the need for external tooling.

It seems to me that making the output location user-configurable per checker would only invoke the cost of feature maintenance on this library, but otherwise not impose any cost on gem consumers who choose to use default settings. I am admittedly new to this ecosystem so perhaps that cost is higher than I estimate?

Tangentially, I’m curious if you’re able to speak to whether Shopify has a different privacy checking path in mind down the road, or rather is not interested in that style of check at this time?

@rafaelfranca just a friendly bump for the conversation here. I know the conference makes for a busy time, so just looking to resume when you're available ❤️