Group creation and update PR nits and feedback fixes

5 files changed, 26 insertions, 26 deletions

diff --git a/src/ext/Util/ca/scaexec.cpp b/src/ext/Util/ca/scaexec.cpp
index fea60b01..a2ecdaab 100644
--- a/src/ext/Util/ca/scaexec.cpp
+++ b/src/ext/Util/ca/scaexec.cpp
@@ -720,8 +720,7 @@ static HRESULT RemoveGroupInternal(
 
         NET_API_STATUS er = ::NetLocalGroupDel(pwzServerName, wzName);
         hr = HRESULT_FROM_WIN32(er);
-        if (HRESULT_FROM_WIN32(ERROR_NO_SUCH_ALIAS) == hr
-            || HRESULT_FROM_WIN32(NERR_GroupNotFound) == hr) // we wanted to delete it.. and the group doesn't exist.. solved.
+        if (HRESULT_FROM_WIN32(ERROR_NO_SUCH_ALIAS) == hr || HRESULT_FROM_WIN32(NERR_GroupNotFound) == hr) // we wanted to delete it.. and the group doesn't exist.. solved.
         {
             hr = S_OK;
         }
@@ -1303,16 +1302,16 @@ extern "C" UINT __stdcall CreateGroup(
         er = ::NetLocalGroupAdd(pwzServerName, 1, reinterpret_cast<LPBYTE>(&groupInfo1), &dw);
         hr = HRESULT_FROM_WIN32(er);
 
-        if (HRESULT_FROM_WIN32(ERROR_ALIAS_EXISTS) == hr
-            || HRESULT_FROM_WIN32(NERR_GroupExists) == hr)
+        if (HRESULT_FROM_WIN32(ERROR_ALIAS_EXISTS) == hr || HRESULT_FROM_WIN32(NERR_GroupExists) == hr)
         {
             if (SCAG_FAIL_IF_EXISTS & iAttributes)
             {
                 MessageExitOnFailure(hr, msierrGRPFailedGroupCreateExists, "Group (%ls\\%ls) was not supposed to exist, but does", pwzDomain, pwzName);
             }
 
-            hr = S_OK; // Make sure that we don't report this situation as an error
+            // Make sure that we don't report this situation as an error
             // if we fall through the tests that follow.
+            hr = S_OK;
 
             if (SCAG_UPDATE_IF_EXISTS & iAttributes)
             {
@@ -1505,6 +1504,7 @@ extern "C" UINT __stdcall CreateGroupRollback(
             {
                 pwzComment = pwzOriginalComment;
             }
+
             hr = WcaReadIntegerFromCaData(&pwz, &iOriginalAttributes);
             if (FAILED(hr))
             {
@@ -1618,7 +1618,7 @@ LExit:
     return WcaFinalize(er);
 }
 
-HRESULT AlterGroupMembership(bool remove, bool isRollback)
+HRESULT AlterGroupMembership(BOOL fRemove, BOOL fIsRollback)
 {
     HRESULT hr = S_OK;
     NET_API_STATUS er = ERROR_SUCCESS;
@@ -1663,7 +1663,7 @@ HRESULT AlterGroupMembership(bool remove, bool isRollback)
     hr = WcaReadStringFromCaData(&pwz, &pwzScriptKey);
     ExitOnFailure(hr, "failed to read scriptkey from custom action data");
 
-    if (isRollback)
+    if (fIsRollback)
     {
         // if the script file doesn't exist, then we'll abandon this rollback
         hr = WcaCaScriptOpen(WCA_ACTION_INSTALL, WCA_CASCRIPT_ROLLBACK, FALSE, pwzScriptKey, &hRollbackScript);
@@ -1693,7 +1693,7 @@ HRESULT AlterGroupMembership(bool remove, bool isRollback)
     }
     memberInfo3.lgrmi3_domainandname = pwzChildFullName;
 
-    if (remove)
+    if (fRemove)
     {
         er = ::NetLocalGroupDelMembers(pwzServerName, pwzParentName, 3, (LPBYTE)&memberInfo3, 1);
     }
@@ -1705,18 +1705,16 @@ HRESULT AlterGroupMembership(bool remove, bool isRollback)
 
     // if there was no error, the action succeeded, and we should flag that it's something which might need
     // to be rolled back
-    if (S_OK == hr && !isRollback)
+    if (S_OK == hr && !fIsRollback)
     {
         // we create a script file, the rollback matching this scriptkey will occur if the file exists
         hr = WcaCaScriptCreate(WCA_ACTION_INSTALL, WCA_CASCRIPT_ROLLBACK, FALSE, pwzScriptKey, FALSE, &hRollbackScript);
         WcaCaScriptClose(hRollbackScript, WCA_CASCRIPT_CLOSE_PRESERVE);
     }
 
-    if (remove)
+    if (fRemove)
     {
-        if (HRESULT_FROM_WIN32(NERR_GroupNotFound) == hr
-            || HRESULT_FROM_WIN32(ERROR_NO_SUCH_MEMBER) == hr
-            || HRESULT_FROM_WIN32(ERROR_MEMBER_NOT_IN_ALIAS) == hr)
+        if (HRESULT_FROM_WIN32(NERR_GroupNotFound) == hr || HRESULT_FROM_WIN32(ERROR_NO_SUCH_MEMBER) == hr || HRESULT_FROM_WIN32(ERROR_MEMBER_NOT_IN_ALIAS) == hr)
         {
             hr = S_OK;
         }
@@ -1771,7 +1769,7 @@ extern "C" UINT __stdcall AddGroupMembership(
     ExitOnFailure(hr, "failed to initialize COM");
     fInitializedCom = TRUE;
 
-    hr = AlterGroupMembership(false, false);
+    hr = AlterGroupMembership(FALSE, FALSE);
 
 LExit:
     if (fInitializedCom)
@@ -1807,7 +1805,7 @@ extern "C" UINT __stdcall AddGroupMembershipRollback(
     ExitOnFailure(hr, "failed to initialize COM");
     fInitializedCom = TRUE;
 
-    hr = AlterGroupMembership(true, true);
+    hr = AlterGroupMembership(TRUE, TRUE);
 
 LExit:
     if (fInitializedCom)
@@ -1842,13 +1840,14 @@ extern "C" UINT __stdcall RemoveGroupMembership(
     ExitOnFailure(hr, "failed to initialize COM");
     fInitializedCom = TRUE;
 
-    hr = AlterGroupMembership(true, false);
+    hr = AlterGroupMembership(TRUE, FALSE);
 
 LExit:
     if (fInitializedCom)
     {
         ::CoUninitialize();
     }
+
     return WcaFinalize(FAILED(hr) ? ERROR_INSTALL_FAILED : ERROR_SUCCESS);
 }
 
@@ -1878,7 +1877,7 @@ extern "C" UINT __stdcall RemoveGroupMembershipRollback(
     ExitOnFailure(hr, "failed to initialize COM");
     fInitializedCom = TRUE;
 
-    hr = AlterGroupMembership(false, true);
+    hr = AlterGroupMembership(FALSE, TRUE);
 
 LExit:
     if (fInitializedCom)
diff --git a/src/ext/Util/ca/scasched.cpp b/src/ext/Util/ca/scasched.cpp
index d7378b13..cef7fecb 100644
--- a/src/ext/Util/ca/scasched.cpp
+++ b/src/ext/Util/ca/scasched.cpp
@@ -134,7 +134,7 @@ extern "C" UINT __stdcall ConfigureGroups(
     __in MSIHANDLE hInstall
 )
 {
-    AssertSz(0, "Debug ConfigureGroups");
+    //AssertSz(0, "Debug ConfigureGroups");
 
     HRESULT hr = S_OK;
     UINT er = ERROR_SUCCESS;
diff --git a/src/ext/Util/wixext/UtilCompiler.cs b/src/ext/Util/wixext/UtilCompiler.cs
index 4b1e43b5..f59ffcd6 100644
--- a/src/ext/Util/wixext/UtilCompiler.cs
+++ b/src/ext/Util/wixext/UtilCompiler.cs
@@ -1485,7 +1485,7 @@ namespace WixToolset.Util
                     switch (child.Name.LocalName)
                     {
                         case "GroupRef":
-                            this.ParseGroupRefElement(intermediate, section, child,  id.Id, groupType:true);
+                            this.ParseGroupRefElement(intermediate, section, child, id.Id, groupType: true);
                             break;
                         default:
                             this.ParseHelper.UnexpectedElement(element, child);
@@ -1521,7 +1521,7 @@ namespace WixToolset.Util
         /// <param name="element">Element to parse.</param>
         /// <param name="childId">Required child id to be joined to the group.</param>
         /// <param name="groupType">whether the child is a group (true) or a user (false)</param>
-        private void ParseGroupRefElement(Intermediate intermediate, IntermediateSection section, XElement element, string childId, bool groupType=false)
+        private void ParseGroupRefElement(Intermediate intermediate, IntermediateSection section, XElement element, string childId, bool groupType)
         {
             var sourceLineNumbers = this.ParseHelper.GetSourceLineNumbers(element);
             string groupId = null;
@@ -3588,7 +3588,7 @@ namespace WixToolset.Util
                                 this.Messaging.Write(UtilErrors.IllegalElementWithoutComponent(childSourceLineNumbers, child.Name.LocalName));
                             }
 
-                            this.ParseGroupRefElement(intermediate, section, child, id.Id, groupType:false);
+                            this.ParseGroupRefElement(intermediate, section, child, id.Id, groupType: false);
                             break;
                         default:
                             this.ParseHelper.UnexpectedElement(element, child);
diff --git a/src/test/burn/WixTestTools/RuntimeFactAttribute.cs b/src/test/burn/WixTestTools/RuntimeFactAttribute.cs
index 76004f26..d7f56f70 100644
--- a/src/test/burn/WixTestTools/RuntimeFactAttribute.cs
+++ b/src/test/burn/WixTestTools/RuntimeFactAttribute.cs
@@ -10,6 +10,8 @@ namespace WixTestTools
 
     public class RuntimeFactAttribute : SkippableFactAttribute
     {
+        private bool domainRequired;
+
         const string RequiredEnvironmentVariableName = "RuntimeTestsEnabled";
         const string RequiredDomainEnvironmentVariableName = "RuntimeDomainTestsEnabled";
 
@@ -47,17 +49,16 @@ namespace WixTestTools
             RuntimeDomainTestsEnabled = Boolean.TryParse(domainTestsEnabledString, out var domainTestsEnabled) && domainTestsEnabled;
         }
 
-        private bool _domainRequired;
         public bool DomainRequired
         {
             get
             {
-                return _domainRequired;
+                return this.domainRequired;
             }
             set
             {
-                _domainRequired = value;
-                if (_domainRequired && String.IsNullOrEmpty(this.Skip) && (!RunningInDomain || !RuntimeDomainTestsEnabled))
+                this.domainRequired = value;
+                if (this.domainRequired && String.IsNullOrEmpty(this.Skip) && (!RunningInDomain || !RuntimeDomainTestsEnabled))
                 {
                     this.Skip = $"These tests require the test host to be running as a domain member ({(RunningInDomain ? "passed" : "failed")}). These tests affect both MACHINE AND DOMAIN state. To accept the consequences, set the {RequiredDomainEnvironmentVariableName} environment variable to true ({(RuntimeDomainTestsEnabled ? "passed" : "failed")}).";
                 }
diff --git a/src/test/burn/WixTestTools/UserGroupVerifier.cs b/src/test/burn/WixTestTools/UserGroupVerifier.cs
index 52a1a6bf..8c2da46e 100644
--- a/src/test/burn/WixTestTools/UserGroupVerifier.cs
+++ b/src/test/burn/WixTestTools/UserGroupVerifier.cs
@@ -29,7 +29,7 @@ namespace WixTestTools
         }
 
         /// <summary>
-        /// Deletes a local gorup from the machine
+        /// Deletes a local group from the machine
         /// </summary>
         /// <param name="groupName">group name to delete</param>
         /// <remarks>Has to be run as an Admin</remarks>