-
Notifications
You must be signed in to change notification settings - Fork 147
Improving minimization by grouping results based on arns #252
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
Conversation
@kmcquade can you review this PR 😄 |
test/writing/test_minimize_write.py
Outdated
"extra actions are returned") | ||
self.assertEqual(write_format['Statement'][1]['Resource'], cfg['write'], "Wrong resources were returned") | ||
|
||
if __name__ == '__main__': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kvsaikiran I dont think this main is required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test/writing/test_minimize_write.py
Outdated
import unittest | ||
from policy_sentry.writing.sid_group import SidGroup | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why a new file
there is already a test file for minimize https://github.com/salesforce/policy_sentry/blob/master/test/writing/test_minimize.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. It should be in the existing test file
I love this meme lol |
@kmcquade @reetasingh please review again. |
What does this PR do?
#235
This will will further minimise the policy output by grouping the similar arns.
What gif best describes this PR or how it makes you feel?
Completion checklist