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

Use the library name from the POM file of the dependency #191

Merged
merged 11 commits into from
Aug 25, 2021

Conversation

rekire
Copy link
Contributor

@rekire rekire commented Jul 3, 2021

I want to have the clean name of the dependency if set in the POM file. My PR will use the name and will fallback when the name tag is not set in the POM file to the artifact group and artifact id.
Fixes #184

Copy link
Collaborator

@caller9 caller9 left a comment

Choose a reason for hiding this comment

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

This CL breaks unit tests in LicenseTaskTest. Please fix existing tests and add tests that verify the new functionality works as intended.

@rekire
Copy link
Contributor Author

rekire commented Aug 13, 2021

I will fix the tests asap, tbh I forgot to run them

@rekire
Copy link
Contributor Author

rekire commented Aug 13, 2021

I fixed the tests and your remarks. There was one broken test file (groupB/bcd/deps2.pom) is was a copy of groupA/deps1.pom instead of a different test case, I fixed that. Could you please review why I had to change the expected output in testAddLicensesFromPom_withMultiple to pass the test? I do not get why this was required.

Thank you for the review!

@rekire
Copy link
Contributor Author

rekire commented Aug 22, 2021

Changed the code and merged my other PR into this one. I'm looking forward new feedback

@caller9 caller9 merged commit d9efd26 into google:master Aug 25, 2021
@rekire rekire deleted the use_dependency_names branch August 25, 2021 18:41
@rekire
Copy link
Contributor Author

rekire commented Nov 16, 2021

@caller9 when I see this correctly then has my patch already landed in multiple Google Apps like the PlayStore, is there any timeframe when I can expect to see this in a public release?

@rekire
Copy link
Contributor Author

rekire commented Mar 17, 2022

My patch went live in 0.10.5 on February 28

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.

Where do the clean short names of the libraries come from?
2 participants