Fix Install-Selected-Packages to update the config #690
Conversation
install.ps1
Outdated
| # Select the package node based on the 'name' attribute | ||
| $nodePackage = $configXml.SelectSingleNode("//packages/package[@name='$package']") | ||
| # if the package checkbox is checked and the package node does not exist in the config | ||
| if ($checkBox.Checked -and (-not $nodePackage)){ |
There was a problem hiding this comment.
I find this logic quite complex and difficult to maintain without introducing bugs. Wouldn't it be easier to generate the config file from scratch (new config with the selected packages) that comparing every node with the config?
There was a problem hiding this comment.
I implemented it in an easier way initially, I restored that version for your review.
Also you will need to delete only the packages but not the envs so I don't think its a good idea to write a new config from the scratch, let me know
There was a problem hiding this comment.
with "from scratch" I meant that you remove everything inside the <packages> tag and then add the selected packages.
sara-rn
force-pushed
the
bug-update-selected-packages
branch
3 times, most recently
from
b97d231 to
d29a434
Compare
sara-rn
changed the title
Fix bug where new selected packages were added but existent packages that had been removed (unchecked) were not deleted from the config.
sara-rn
force-pushed
the
bug-update-selected-packages
branch
from
d29a434 to
cb0a8dd
Compare
There was a problem hiding this comment.
This version is already simpler than the previous one, but I think it can still be simplified more, making it easier to read and maintain, preventing bugs in the future: Delete everything inside the <packages> tag and then add the selected packages. This way you don't need to save and iterate the $unselectedPackages.
sara-rn
force-pushed
the
bug-update-selected-packages
branch
2 times, most recently
from
3f30869 to
3dde7a7
Compare
Remove all the entries inside the <packages> node and adds the selected packages.
sara-rn
force-pushed
the
bug-update-selected-packages
branch
from
3dde7a7 to
e4dce7e
Compare
Fix bug where new selected packages were added but existent packages that had been removed (unchecked) were not deleted from the config.
closes #681