[REVIEW please] CreateGroups additions #530
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bevanweiss
commented
Jun 19, 2024
bevanweiss
commented
Jun 19, 2024
bevanweiss
commented
Jun 19, 2024
bevanweiss
commented
Jun 19, 2024
bevanweiss
commented
Jun 19, 2024
bevanweiss
commented
Jun 19, 2024
bevanweiss
force-pushed
the
util_creategroup
branch
8 times, most recently
from
871c524
to
06e7fa6
Compare
bevanweiss
force-pushed
the
util_creategroup
branch
3 times, most recently
from
15346cb
to
fc0bf93
Compare
bevanweiss
force-pushed
the
util_creategroup
branch
from
fc0bf93
to
8dad7d5
Compare
|
@barnson Are there any additional integration tests that you think would be good to have beyond what I've already got in this?
I know there's a heap of code in this PR, however there's not really many easily separable portions that would be meaningful on their own. However, let me know if there's any way you can see that I could split it into more easily reviewable/pull-able portions. |
|
I'm not a domain expert but the tests look reasonable to me. |
Signed-off-by: Bevan Weiss <bevan.weiss@gmail.com>
Local group membership Add/Remove working, however with BUILTIN local system groups .NET doesn't appear to locate them as either groups nor basic security Principals. Still needs work to fix the test for nested groups. Ideally with some way to test for domain groups. Signed-off-by: Bevan Weiss <bevan.weiss@gmail.com>
Signed-off-by: Bevan Weiss <bevan.weiss@gmail.com>
Fixups to a few test cases. Signed-off-by: Bevan Weiss <bevan.weiss@gmail.com>
bevanweiss
force-pushed
the
util_creategroup
branch
from
53691b4
to
f507768
Compare
bevanweiss
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just a placeholder for the current state of development on the ability to add Groups. wixtoolset/issues#8577
Not sure if it works yet, splitting out the tables cost me a little more time than I anticipated, and I'm unsure that the decompile is right.
I'll build out a few more of the build test cases, to validate the expected preconditions etc (i.e. marking CreateGroup when it's not under a component, and similar).
Please raise any style / formatting issues, that way I've got the most time to address them :)
I've just noticed I've missed a couple of Wix4-.Wix6 references (around CustomAction names). I'll tidy these up tomorrow.
Closes wixtoolset/issues#8577