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

Remove toLower on source in win_chocolatey module #31983

Merged
merged 1 commit into from
Oct 22, 2017
Merged

Remove toLower on source in win_chocolatey module #31983

merged 1 commit into from
Oct 22, 2017

Conversation

keslerm
Copy link
Contributor

@keslerm keslerm commented Oct 20, 2017

Having this here breaks any source URLs that require case sensitivity.

SUMMARY

Removes the .toLower from the chocolatey source because it breaks source URLs that may be case sensitive.

For example some private nuget repos use a mixed case string as an identifier that is required to match casing exactly.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

windows/win_chocolatey

ANSIBLE VERSION
ansible 2.2.0.0

Having this here breaks any source URLs that require case sensitivity.
@ansibot
Copy link
Contributor

ansibot commented Oct 20, 2017

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 bugfix_pull_request community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. windows Windows community labels Oct 20, 2017
@jhawkesworth
Copy link
Contributor

This looks like a sensible change to me, just curious why it was added in the first place.

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Oct 20, 2017
@keslerm
Copy link
Contributor Author

keslerm commented Oct 20, 2017

@jhawkesworth I checked the PR it came in with and it doesn't really elude to why it was included - #26523 - so if there was a specific reason (though I can't really think of any) it was unfortunately not documented.

@dagwieers
Copy link
Contributor

dagwieers commented Oct 21, 2017

@keslerm It was already included before that PR, so I cannot help why it was done. And indeed undocumented.

This line was added in commit cb88c17

@jborean93
Copy link
Contributor

We should not be setting toLower() on the URL as it breaks RFC, thanks for the fix @keslerm.

@jborean93 jborean93 merged commit ed342e8 into ansible:devel Oct 22, 2017
@jborean93 jborean93 added this to TODO: Next release in 2.4.x Blocker List Oct 22, 2017
@abadger
Copy link
Contributor

abadger commented Oct 26, 2017

cherrypicked for the 2.4.2beta1 release.

@abadger abadger moved this from TODO: Next release to Done in 2.4.2 in 2.4.x Blocker List Oct 26, 2017
abadger pushed a commit that referenced this pull request Oct 26, 2017
Having this here breaks any source URLs that require case sensitivity.
(cherry picked from commit ed342e8)
abadger pushed a commit that referenced this pull request Oct 26, 2017
Having this here breaks any source URLs that require case sensitivity.
(cherry picked from commit ed342e8)
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.5 This issue/PR affects Ansible v2.5 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. windows Windows community
Projects
No open projects
2.4.x Blocker List
Done in 2.4.2
Development

Successfully merging this pull request may close these issues.

None yet

6 participants