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

Fixed bug in generating distributionUrl #1408

Closed
wants to merge 2 commits into from
Closed

Conversation

er-han
Copy link
Contributor

@er-han er-han commented Feb 15, 2017

Bug was only in those locales which contain small dotless I letter (ı).
More detail: https://groups.google.com/forum/#!topic/gradle-dev/JHZqo_EJ2zs

Any of the checked boxes below indicate that I took action:

For all non-trivial changes that modify the behavior or public API:

  • Before submitting a pull request, I started a discussion on the Gradle developer list,
    the forum or can reference a JIRA issue.
  • I considered writing a design document. A design document can be
    brief but explains the use case or problem you are trying to solve,
    touches on the planned implementation approach as well as the test cases
    that verify the behavior. Optimally, design documents should be submitted
    as a separate pull request. Samples
    can be found in the Gradle GitHub repository. Please let us know if you need help with
    creating the design document. We are happy to help!
  • The pull request contains an appropriate level of unit and integration
    test coverage to verify the behavior. Before submitting the pull request
    I ran a build on your local machine via the command
    ./gradlew quickCheck <impacted-subproject>:check.
  • The pull request updates the Gradle documentation like user guide,
    DSL reference and Javadocs where applicable.

Bug was only in those locales which contain small dotless I letter.
@lacasseio
Copy link
Contributor

Thank you very much for providing a PR for this issue. Would you mind adding a test for this PR just so we don't break this use case in the future?

@er-han
Copy link
Contributor Author

er-han commented Feb 16, 2017

@lacasseio I have added a test for this bug fix.

@lacasseio
Copy link
Contributor

lacasseio commented Mar 3, 2017

Thanks for the contribution. I manually merge it as I did a small fixed to the test. We identified that other places in Gradle code base is vulnerable this toLowerCase problem. If you feel strongly about this, we could use your help in improving Gradle. Have a look at issue #1506. We would be looking at adding Findbugs or Pmd check to catch this kind of problem and fixing every occurrence.

@lacasseio lacasseio closed this Mar 3, 2017
@er-han
Copy link
Contributor Author

er-han commented Mar 9, 2017

I would be glad if I can help in improving Gradle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants