(GH-293) choco upgrade all -except="package1,package2" #343
Conversation
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This should roll in under the upgrade scenarios. Thoughts?
There was a problem hiding this comment.
That was my first attempt, but the Context (the tests setup) was very different, so I thought it'd be easier to understand if it was separate. Merging the two would require a refactor of UpgradeScenarios that goes beyond the PR.
There was a problem hiding this comment.
Why I consider this refactoring outside of the PR? Because there was no test initially (that I could find) that tested "upgrade all", so I added some but the choco upgrade all command already existed prior to that PR.
Also, the upgrade tests all shared a common "context" (test setup), so changing that would require refactoring the whole UpgradeScenarios class, changing each test or restructuring them... but the PR only focus on filtering choco upgrade all packages, so changing this file completely - adding to this eventual merge issues if the PR is not quickly merged - falls outside of the actual except feature.
I tried to keep the new file as clean and simple as possible so that if eventually UpgradeScenarios and UpgradeAllScenarios were to be merged, there would be minimal headaches.
There was a problem hiding this comment.
Also, the upgrade tests all shared a common "context" (test setup), so changing that would require refactoring the whole UpgradeScenarios class, changing each test or restructuring them... but the PR only focus on filtering choco upgrade all packages, so changing this file completely - adding to this eventual merge issues if the PR is not quickly merged - falls outside of the actual except feature.
Well they share it, but they can override it in each individual spec class. And if you look through many of them, they do. Some override and still call the base, although not required. The way it is setup is quite flexible, no refactoring required.
|
Overall +1 |
|
Feedback included and merge complete. Is there anything to do for documentation, or will you be taking care of this? |
|
Documentation, yes, Would need to include some information in the help method related to this. |
|
I added some documentation to the |
| @@ -97,7 +97,7 @@ public IEnumerable<PackageResult> list_run(ChocolateyConfiguration config) | |||
| if (config.RegularOutput) this.Log().Debug(() => "Running list with the following filter = '{0}'".format_with(config.Input)); | |||
| if (config.RegularOutput) this.Log().Debug(() => "--- Start of List ---"); | |||
| foreach (var pkg in NugetList.GetPackages(config, _nugetLogger)) | |||
|
|
|||
There was a problem hiding this comment.
This was from the merge, and was not my change (I should have just rebased)
There was a problem hiding this comment.
actually a rebase is what we always hope for. I don't like merge commits anywhere except for explicit merges.
There was a problem hiding this comment.
Understandable, I'd ask the same. I was lazy, I'll rebase as soon as I can :)
|
On the wiki, not at the current time. Sometimes when we add something new, we mention what version it is coming out in. |
christianrondeau
force-pushed
the
293-all-except
branch
from
b47dd09 to
1b6821f
Compare
|
Squashed and rebased on |
christianrondeau
force-pushed
the
293-all-except
branch
from
1b6821f to
05cd7e7
Compare
christianrondeau
force-pushed
the
293-all-except
branch
from
05cd7e7 to
d8a258c
Compare
christianrondeau
changed the title
|
Merged the UpgradeScenarios and UpgradeAllScenarios, ready to merge |
|
Rebased, commit message fixed up and merged into stable. |
This pull request implements #293 by providing a list of packages to ignore.
These packages will not be upgraded because they were specified in '-except': package1when specifying packages to skip-exceptdo not exist in the installed list of packages, it will display a warning:Some packages specified in '-except' were not found in the local packages: 'package1'-exceptargument is not provided, there is no differencechoco upgrade allthat I could find, so I added a very basic one, based on the fact that upgrades are already tested inUpgradeScenarios-skipbut it seems-s*will be catched by-source, so I stucked with-exceptHopefully, all will be good with that pull request :)
Fixes #293.