refactor(acl): consolidate acl module#292
refactor(acl): consolidate acl module#292IamLunchbox merged 13 commits intoansible-collections:mainfrom
Conversation
Thulium-Drake
left a comment
Thulium-Drake
left a comment
There was a problem hiding this comment.
Apart from the thing about the default value, LGTM!
| if roleid is not None and ace["roleid"] != roleid: | ||
| return False | ||
| ace_type = desired.get("type") | ||
| if ace_type is not None and ace["type"] != ace_type: |
There was a problem hiding this comment.
| if ace_type is not None and ace["type"] != ace_type: | |
| if ace_type and ace["type"] != ace_type: |
PendaGTP
force-pushed
the
feat/consolidate-acl-module
branch
from
681c662 to
f26a45d
Compare
|
@IamLunchbox @Thulium-Drake Thanks for review! I have also rebased. Done ✔️ |
|
Thank you for your help :) |
| @@ -0,0 +1,3 @@ | |||
| --- | |||
| breaking_changes: | |||
| - proxmox - set ``state`` as not ``optional`` and assign default value ``present`` (https://github.com/ansible-collections/community.proxmox/pull/292). | |||
There was a problem hiding this comment.
I think it should have been "not required", and not "not optional" :) That's also not a breaking change, since any previous use of the parameter is still valid.
There was a problem hiding this comment.
@felixfontein Sorry, should I do anything to "revert" that? Or it too late?
There was a problem hiding this comment.
Don't worry :)
You can just create a new PR to edit this fragment. It will be parsed on release.
There was a problem hiding this comment.
This fragment seems to be already added to changelog.md on the 1.6.0 release :/ Is it still possible to edit/revert it?
There was a problem hiding this comment.
It's already part of a release, but you can still edit it here: https://github.com/ansible-collections/community.proxmox/blob/main/changelogs/changelog.yaml#L304-L306 Afterwards run antsibull-changelog generate to update CHANGELOG.md and CHANGELOG.rst.
There was a problem hiding this comment.
@felixfontein Thanks for your help, done here #370
4 participants
SUMMARY
This PR consolidate the ACL module.
Bellow main changes:
This works is related to other changes/refactor we have done.
Note: order of PR merged is not important to me, I will update and rebase when changes merged
EDIT: I will add check/diff mode and update tests after, I did'nt introduce it here, to keep the PR smaller
COMPONENT NAME
proxmox_access_acl