-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add explicit acl rule to satisfy Issue #1649 #1966
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1966 +/- ##
==========================================
- Coverage 89.6% 89.58% -0.02%
==========================================
Files 245 246 +1
Lines 14360 14451 +91
Branches 922 929 +7
==========================================
+ Hits 12867 12946 +79
- Misses 1479 1490 +11
- Partials 14 15 +1
Continue to review full report at Codecov.
|
@jpsim I think the thousands of warnings this rule just raised on those sample projects is proof of how pervasive in the swift community it is too ignore the ACLs. I am uncertain however, why it did run on them, since I overrode OptInRule... Is there anything specific I should do so that it doesn't run on that job? |
@joseprl89 The oss-check runs all rules to make sure we are properly testing all of them, no matter whether they're opt-in or if the project has disabled some of them. |
In that case, we have a legit 16k warnings on those projects apparently... Looks rather noisy :S |
I'm fine with merging this considering it's opt-in. Thanks for your contribution @joseprl89! |
Awesome! Thanks a lot @jpsim! Looking forward to start using it in production :) |
@jpsim Implemented a rule to enforce explicit acls not only on the top level, based on explicit_top_level_acl.
Basically:
Will trigger a warning on the let b = 5 to ensure that proper class encapsulation is not forgotten.
Tests seem to pass OK, and I updated the changelog file as proposed in the contributing guidelines, so I believe everything is in order. Let me know if anything is amiss.