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

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

Open
jakebrady5 opened this issue Apr 18, 2023 · 4 comments

Comments

@jakebrady5
Copy link

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.

@jakebrady5 jakebrady5 changed the title [Feature Reqeust] Allow configuration of package_todo.yml violation destinations [Feature Request] Allow configuration of package_todo.yml violation destinations Apr 18, 2023
@professor
Copy link

professor commented Apr 19, 2023

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.

@rafaelfranca
Copy link
Member

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.

@jakebrady5
Copy link
Author

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?

@jakebrady5
Copy link
Author

@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 ❤️

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