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.

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

(#72) Add ability to include sources from config #2649

Merged
merged 2 commits into from
Apr 25, 2024

Conversation

TheCakeIsNaOH
Copy link
Member

@TheCakeIsNaOH TheCakeIsNaOH commented Mar 11, 2022

Description Of Changes

This adds the --include-configured-sources switch which will append
any sources in the config to manually specified sources (via --source).
It checks if the source already exists before appending, which prevents
duplication when the manually specified sources and the sources from
config are combined.

Motivation and Context

When testing a package locally, it may have depencies that are not installed, and available on a remote repository, like the Community Repository. This allows installation/upgrading of the package without having to specify that remote repository explicitly in --source, assuming it is in the sources configured with the source command.

Testing

Validate that the switch is ignored when using an alternate source:
.\choco.exe install nonexistant-package --source=python --include-configured-sources
Output should include this message: Not including sources from config because python is an alternate source

Validate that config sources are not included when not using switch:
.\choco.exe install nonexistant-package --source .
Output error should specify source as '.', and not include the default community repository.

Validate that config sources are included when using switch:
.\choco.exe install nonexistant-package --source . --include-configured-sources
Output error should specify sources as '.;https://community.chocolatey.org/api/v2/'

Validate that config sources are not duplicated if included both in --source and config:
.\choco.exe install nonexistant-package --source="https://community.chocolatey.org/api/v2/" --include-configured-sources
Output error should specify source as 'https://community.chocolatey.org/api/v2/'

Change Types Made

  • Bug fix (non-breaking change)
  • Feature / Enhancement (non-breaking change)
  • Breaking change (fix or feature that could cause existing functionality to change)

Related Issue

Fixes #72

Change Checklist

  • Requires a change to the documentation
  • Documentation has been updated
  • Tests to cover my changes, have been added
  • All new and existing tests passed.
  • N/A PowerShell v2 compatibility checked.

@pauby
Copy link
Member

pauby commented Mar 11, 2022

The name of the switch --include-machine-sources I don't feel is consistent with other switches. We don't use the word machine in any other switches so it's not intuitive.

The issue talks about --include-default-sources but given the work so make a source a default push source, that may also be confusing.

@TheCakeIsNaOH
Copy link
Member Author

The name of the switch --include-machine-sources I don't feel is consistent with other switches. We don't use the word machine in any other switches so it's not intuitive.

That is what the configured sources are called internally in the code, which is why I picked that.

The issue talks about --include-default-sources but given the work so make a source a default push source, that may also be confusing.

Fair enough, that is pulled from the original issue.

I'm completely open to options for the name of the switch.

@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch from 3846706 to c3507da Compare March 21, 2022 16:32
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch from c3507da to ba9d347 Compare May 8, 2022 02:42
@TheCakeIsNaOH TheCakeIsNaOH changed the title (#72) Add ability to include machine sources (#72) Add ability to include sources from config May 10, 2022
@TheCakeIsNaOH TheCakeIsNaOH marked this pull request as ready for review May 10, 2022 20:23
@TheCakeIsNaOH
Copy link
Member Author

I've updated the switch name to --include-sources-from-config

@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch from ba9d347 to 21f2ca0 Compare May 10, 2022 20:23
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch from 21f2ca0 to b69fbad Compare June 27, 2022 14:56
@coveralls
Copy link

coveralls commented Jun 27, 2022

Coverage Status

Coverage remained the same at 27.611% when pulling ab36aac on TheCakeIsNaOH:include-machine-sources into 3892cfb on chocolatey:develop.

@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch 3 times, most recently from 802b076 to 7d0364b Compare August 12, 2022 13:30
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch 2 times, most recently from 1ed4d11 to ab36aac Compare September 23, 2022 05:19
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch from ab36aac to 661fe5e Compare December 22, 2022 23:24
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch 2 times, most recently from c850157 to 2b4621c Compare January 13, 2023 15:25
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch from 2b4621c to a7c9558 Compare June 16, 2023 01:50
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch 2 times, most recently from 4750e0f to a74edc7 Compare July 1, 2023 03:19
@gep13
Copy link
Member

gep13 commented Dec 14, 2023

What about --include-configured-sources for the option name?

@TheCakeIsNaOH
Copy link
Member Author

What about --include-configured-sources for the option name?

Sounds good to me. Should I switch it over to that?

@TheCakeIsNaOH TheCakeIsNaOH force-pushed the include-machine-sources branch 2 times, most recently from 628346d to 8ad3464 Compare January 9, 2024 22:16
TheCakeIsNaOH and others added 2 commits April 25, 2024 06:02
This adds the --include-configured-sources switch which will append
any sources in the config to manually specified sources (via --source).
It checks if the source already exists before appending, which prevents
duplication when the manually specified sources and the sources from
config are combined.
Alternate Sources -> Alternative Sources
@gep13 gep13 force-pushed the include-machine-sources branch from 8ad3464 to c2e08b0 Compare April 25, 2024 13:10
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13
Copy link
Member

gep13 commented Apr 25, 2024

@TheCakeIsNaOH thanks, again, for all your work here! Really appreciate it!

@gep13 gep13 merged commit 04ae937 into chocolatey:develop Apr 25, 2024
5 checks passed
@TheCakeIsNaOH TheCakeIsNaOH deleted the include-machine-sources branch April 26, 2024 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow the ability to add the configured sources when using the --source option
4 participants