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

Introducing enforcement_globs_ignore #189

Merged
merged 3 commits into from
May 22, 2024

Conversation

perryqh
Copy link
Contributor

@perryqh perryqh commented May 21, 2024

What

The ability to ignore violations by glob patterns.

enforcement_globs_ignore:
- enforcements:
  - privacy
  - visiblity
  ignores:
  - "**/*"
  - "!packs/product_services/foo/**"

The above configuration will enforce privacy and visibility violation references only in packs/product_services/foo

Why

We have groupings of packs that we call product_services. Inside a product service, we don't care about privacy violations. We need a mechanism to enforce_privacy, but only for packs outside of the product service.

This also gives us the ability to lock down a pack with strict even if there are offending packs, which can prevent new violations.

Notes

This might be a break from packwerk as we don't plan on immediately making this change to packwerk-extensions. It might not make sense to merge this.

if serialized_pack == "{}\n" {
"".to_owned()
} else {
serialized_pack.replace("\n-", "\n -")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this line because there's 2 levels of nesting.

Copy link
Owner

@alexevanczuk alexevanczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So @perryqh is this basically a more configurable version of the ignore field in packwerk.yml? So a pack can say that it basically opts out of any violations related to references it makes in some files it owns?

@perryqh
Copy link
Contributor Author

perryqh commented May 21, 2024

So @perryqh is this basically a more configurable version of the ignore field in packwerk.yml? So a pack can say that it basically opts out of any violations related to references it makes in some files it owns?

Yes! A pack can also ignore outgoing violations. This change gives finer grain control of enforce_<violation>

Copy link
Owner

@alexevanczuk alexevanczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Only thing I'm not 100% sold on is the name of enforcement_globs_ignore. The language is not super clear to me but I can't think of a better alternative right now 🤷🏼

Separately, the naming is enforce_xyz... but we also refer to these as checkers. I think since we already use the enforce language it's fine but now that I'm thinking of it I think I wish packwerk just used "check" everywhere, e.g. check_dependencies, check_privacy, etc. Not in scope here though...

fn folder_visible(from_pack: &Pack, to_pack: &Pack) -> Result<bool> {
if to_pack.enforce_folder_visibility().is_false() {
return Ok(true);
fn folder_visible(referencing_pack: &Pack, defining_pack: &Pack) -> bool {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice name change

use super::*;

#[macro_export]
macro_rules! test_ignore {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dang, you're fancy with these macros.

I haven't gotten into the rust macros yet. I've never liked meta-programming in Ruby, but mostly because it's hard to verify. I love how Rust's meta-programming is statically type checked!

@alexevanczuk
Copy link
Owner

@perryqh This is in draft mode. Let me know when it's ready to go and I'll merge it!

@perryqh
Copy link
Contributor Author

perryqh commented May 21, 2024

Thanks for the review @alexevanczuk! I'm going to do a lot more testing today before I mark it as "ready for review". If you think of better names, please let me know!

@perryqh perryqh force-pushed the ph/enforce-globs-ignore branch 6 times, most recently from 46e2c40 to 4fd7c4c Compare May 21, 2024 19:15
@perryqh perryqh force-pushed the ph/enforce-globs-ignore branch from 4fd7c4c to 16b4838 Compare May 21, 2024 19:52
@perryqh perryqh marked this pull request as ready for review May 21, 2024 21:43
@perryqh
Copy link
Contributor Author

perryqh commented May 21, 2024

Ready for review @alexevanczuk. Thanks!

@perryqh perryqh requested a review from alexevanczuk May 22, 2024 14:39
@alexevanczuk alexevanczuk merged commit bd8a3d6 into alexevanczuk:main May 22, 2024
6 checks passed
@perryqh perryqh deleted the ph/enforce-globs-ignore branch June 6, 2024 18:00
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

Successfully merging this pull request may close these issues.

2 participants