Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(#357) Add Export Command #1825

Merged
merged 4 commits into from
May 6, 2021
Merged

(#357) Add Export Command #1825

merged 4 commits into from
May 6, 2021

Conversation

gep13
Copy link
Member

@gep13 gep13 commented May 13, 2019

This will allow the creation of a packages.config file of all the
currently installed packages on the machine. Usage of this command
will be:

choco export -o c:/temp/packages.config --include-version-numbers

This command is particularly useful when re-building a machine. i.e.
First export all packages currently installed on machine, and then
replay this packages.config via choco install packages.config on new
machine.

Fixes #357

@gep13 gep13 requested a review from ferventcoder May 13, 2019 08:04
@RichiCoder1
Copy link
Contributor

Does this/could this include pinned packages info? And/or installation/package args?

@gep13
Copy link
Member Author

gep13 commented May 13, 2019

There is no reason that additional features couldn’t be added to this new command. I would see that as a separate enhancement issue personally though, as there would be design decisions that would need to be made around the persisted arguments. @ferventcoder and I have spoken briefly about changes that need to happen in this area, and things would need to be flushed out.

Regarding the pinned package side of things, are you asking that if a pinned is pinned, this makes it into the packages.config file and when replayed on a new machine, that package also ends up being pinned?

@RichiCoder1
Copy link
Contributor

There is no reason that additional features couldn’t be added to this new command. I would see that as a separate enhancement issue personally though, as there would be design decisions that would need to be made around the persisted arguments. @ferventcoder and I have spoken briefly about changes that need to happen in this area, and things would need to be flushed out.

Fair, that makes sense.

Regarding the pinned package side of things, are you asking that if a pinned is pinned, this makes it into the packages.config file and when replayed on a new machine, that package also ends up being pinned?

Exactly. So if I pinned VS Code on one machine, it would end up pinned on another machine.

@gep13
Copy link
Member Author

gep13 commented May 13, 2019

@RichiCoder1 said...
Exactly. So if I pinned VS Code on one machine, it would end up pinned on another machine

In the current implementation, pin is a standalone command, I.e. you can’t pin a package during the process of installation. As such, this may need to be implemented as a second pass of the packages.config file to look for pinned packages, or a modification to the install command to also pin when installing.

Again, more discussion would need to be had on this topic.

ferventcoder
ferventcoder requested changes May 13, 2019
View reviewed changes
Copy link
Member

@ferventcoder ferventcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, there are a couple of things to address. Take a look at those items.

@gep13 gep13 force-pushed the feature/GH-357 branch 2 times, most recently from c888688 to dc18d24 Compare May 31, 2019 12:07
@AdmiringWorm AdmiringWorm mentioned this pull request Jun 23, 2019
Closed
@mwallner
Copy link
Member

mwallner commented Aug 8, 2019

is there a timeline for this feature to become available? :-)

@gep13
Copy link
Member Author

gep13 commented Aug 8, 2019

@mwallner I have some tidy up work that I need to do on this PR before it would be ready to be merged in. I haven't forgotten about it, just need to get round to it. So, to answer your question, no, there isn't a timeline 😄

@gep13 gep13 force-pushed the feature/GH-357 branch 2 times, most recently from a0df909 to 358ded8 Compare May 28, 2020 19:46
@gep13
Copy link
Member Author

gep13 commented May 28, 2020

@ferventcoder I believe this is ready for another review. There is one outstanding question I believe with regard to the use of XmlService, which I am not clear on. If this is required, can you let me know exactly how you see this working? Thanks

@CLAassistant
Copy link

CLAassistant commented Jun 8, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@eabase
Copy link

eabase commented Oct 15, 2020

@ferventcoder
Maybe it's time to merge this?

@gep13 gep13 changed the base branch from master to stable April 20, 2021 15:40
vexx32
vexx32 approved these changes Apr 21, 2021
View reviewed changes
Copy link
Member

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good to me, bar one question I had around the design here: #1825 (comment)

gep13 added 3 commits May 6, 2021 11:00
@gep13
(chocolatey#357) Change modifier for method
a0c679a 
So that the method to get_all_installed_packages can be consumed from
elsewhere in the codebase.  This meant extending the interface for
INugetService to include this method as well, otherwise, a concrete
reference to an instance of NuGetService would be required, which is not
available.
@gep13
(chocolatey#357) Add Export Command
ce6ccb8 
This will allow the creation of a packages.config file of all the
currently installed packages on the machine.  Usage of this command
will be:

choco export -o c:/temp/packages.config --include-version-numbers

This command is particularly useful when re-building a machine.  i.e.
First export all packages currently installed on machine, and then
replay this packages.config via choco install packages.config on new
machine.
@gep13
(chocolatey#357) Add unit tests
65ff904 
To cover the new functionality of the export command. This includes
verifying the option sets, what methods are called under certain
circumstances, etc.
@gep13
Copy link
Member Author

gep13 commented May 6, 2021

@vexx32 I have updated this PR based on your feedback. If you could take another look, I would appreciate it. I have also added some Unit Tests since the last review. Thanks

@vexx32
Copy link
Member

vexx32 commented May 6, 2021

Apart from those two things, everything here looks good to me!

vexx32
vexx32 approved these changes May 6, 2021
View reviewed changes
Copy link
Member

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, nice work! 💖

@gep13 gep13 changed the title (GH-357) Add Export Command (#357) Add Export Command May 6, 2021
@gep13 gep13 merged commit 57f8b6a into chocolatey:stable May 6, 2021
@gep13 gep13 deleted the feature/GH-357 branch May 6, 2021 14:44
@TheCakeIsNaOH TheCakeIsNaOH mentioned this pull request Dec 30, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwallner mwallner mwallner left review comments

@AdmiringWorm AdmiringWorm AdmiringWorm left review comments

@ferventcoder ferventcoder ferventcoder requested changes

@vexx32 vexx32 vexx32 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide ability to export all currently installed packages to a packages.config file
8 participants
@gep13 @RichiCoder1 @mwallner @CLAassistant @eabase @vexx32 @ferventcoder @AdmiringWorm