Colocate privacy API on Packwerk::Package #233
Conversation
alexevanczuk
force-pushed
the
ae-colocate-privacy-api-on-package
branch
from
bdb35a2 to
28b529a
Compare
| @@ -11,6 +11,39 @@ class PrivacyChecker | |||
|
|
|||
| VIOLATION_TYPE = T.let("privacy", String) | |||
|
|
|||
| class << self | |||
There was a problem hiding this comment.
Does it make sense to put this in a nested different class or module? This logic isn't really related to the checker, it belongs to the package.
There was a problem hiding this comment.
Eventually, this whole class will implement a Packwerk::PluginInterface, which will connect it to the rest of the packwerk runtime. I think that it might make sense to better factor out the privacy implementation into more logical parts when it comes time to do that, but I was hoping at this stage to just move all privacy related things into a pile in one place so that it can be deleted from packwerk.
Let me know if you think think that makes sense as a stepping stone.
There was a problem hiding this comment.
I would prefer the logic be encapsulated somewhere else. It is hard for me to visualize the end goal of how the logic will be organized. For now, this is still part of Packwerk for the time being, so perhaps an easy and testable way to do is through a composite object that lives in the privacy namespace?
There was a problem hiding this comment.
Okay! I moved the logic to a PrivacyProtectedPackage.
alexevanczuk
force-pushed
the
ae-colocate-privacy-api-on-package
branch
2 times, most recently
from
16f4479 to
cc41029
Compare
|
@gmcgibbon Can you take another look? |
alexevanczuk
force-pushed
the
ae-colocate-privacy-api-on-package
branch
from
9fda03d to
9120df2
Compare
What are you trying to accomplish?
Much like #232, this moves us closer to #219 by colocating all concerns around privacy on
Packwerk::Packageby putting them inPrivacyCheckerWhat approach did you choose and why?
I did not yet remove the old API on
Packwerk::Package, since that would be a breaking API change, which I'd like to do all at once when we movePrivacyCheckerto its own gem. When that happens, that gem can have a nice and easy to use API that can replace the old APIs folks were using onPackwerk::Package.What should reviewers focus on?
Let me know if this approach makes sense to you!
I'm wondering if I should just go ahead and remove the old API from
Packwerk::Package. While it's probably best to assume folks are using it, I also wouldn't be surprised if very few or no clients are actually using them (packwerkitself is using them in very few places).Type of Change
Checklist