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

Recursively scan parent POMs for license #23

Closed
wants to merge 2 commits into from

Conversation

ChristianCiach
Copy link
Contributor

Previously, only the direct parent POM was scanned for license information. In many cases, this is not sufficient.

This pull request changes this behavior so that all parent POMs are scanned recursively. In my case, this finds A LOT of licenses that weren't found before, like for various Apache-commons and Jetty libraries.

@ChristianCiach
Copy link
Contributor Author

ChristianCiach commented Dec 11, 2017

Sorry, the second commit shouldn't be part of this pull request.

But now that it's done, feel free which commits you want to merge :)

A few words about the second commit: I though it would make sense to include all licenses of a library instead of just the first one. This means that this is now an array inside the generated json. The same is true for the "developers", which are now an array instead of a concatenated string. The json now also contains the description and version of the library, because the license can change with new versions. The "url" of the library is now extracted from pom -> url instead of pom -> scm -> url.

All changes of the second commit only apply to the json report. The html report is exactly the same as before.

@jaredsburrows
Copy link
Owner

jaredsburrows commented Dec 28, 2017

@ChristianCiach So sorry! I never got a email about this. This is a great contribution, thank so much!

  • Can you update the README, with your updated licenses JSON and HTML?
  • Maybe add a test with one of the Square libraries to test the parent POM search?
  • Make sure you use git config user.name <your name> and git config user.email <your email> to make sure you get credit when merging this PR.

@jaredsburrows
Copy link
Owner

@ChristianCiach Let's update this PR real fast and merge in your feature.

@jaredsburrows
Copy link
Owner

@ChristianCiach Here is the issue: #15.

@ChristianCiach
Copy link
Contributor Author

Thanks for the feedback!

I probably can't work on this in the next days, but you can expect the proposed changes next week.

Thanks for the tip with the git configuration. For this I probably need to close this pull request and open a new one.

I will split the two commits to separate pull requests. Are you even interested to merge the second commit? I would appreciate it, because I wouldn't need to maintain a fork in this case. But please keep in mind that this would be a breaking change for existing users depending on the json report.

@jaredsburrows
Copy link
Owner

You can close and create a new one or simply amend authorship: https://stackoverflow.com/a/3042512/950427.

Both commits are great. I honestly do not care if they are in a separate PR or not. We just need to update the README.

@ChristianCiach
Copy link
Contributor Author

Alright, I am back and I will see what I can do :)

@ChristianCiach
Copy link
Contributor Author

Closing pull request in favor of #27 and #29.

@jaredsburrows
Copy link
Owner

@ChristianCiach Sounds good.

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.

2 participants