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

Upgrade license to GPL-3.0-or-later #2949

Conversation

ssoloff
Copy link
Member

@ssoloff ssoloff commented Feb 3, 2018

Per #2764.

Things of note:

  • I placed the license notice (with the exceptions) in the README. This seemed simpler than creating a separate file (e.g. LICENSE-NOTICE) that no one may actually read.
  • I added a copyright notice to the license notice, as recommended in the GPL. I attributed the copyright to "TripleA contributors." Not sure if this is the best choice, but it really becomes moot if we adopt a CLA or a DCO (see Adopt a Contributor License Agreement #2880).
  • The only exception I listed in the license notice was JavaMail. After further research, I concluded that it is probably unnecessary to list exceptions for libraries that we do not actually distribute with our release artifacts (which includes the JUnit 5 and JUnit 5 Samples libraries). If anyone disagrees, it's a simple matter to add the four additional libraries to the exceptions table.
  • I believe the license badge in the README will automatically update once this PR is merged.

Unrelated to #2764:

  • I changed the Markdown headings in the README to use # delimiters instead of underlining them with ===.
  • I updated many links in the README to be canonical (e.g. use www.triplea-game.org instead of triplea-game.github.io), to use HTTPS if possible, etc.

@codecov-io
Copy link

codecov-io commented Feb 3, 2018

Codecov Report

Merging #2949 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2949      +/-   ##
============================================
- Coverage      20.4%   20.39%   -0.01%     
+ Complexity     5770     5769       -1     
============================================
  Files           817      817              
  Lines         72905    72906       +1     
  Branches      12035    12035              
============================================
- Hits          14876    14872       -4     
- Misses        55933    55936       +3     
- Partials       2096     2098       +2
Impacted Files Coverage Δ Complexity Δ
...va/games/strategy/triplea/ui/menubar/HelpMenu.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...main/java/games/strategy/triplea/UrlConstants.java 100% <100%> (ø) 3 <0> (ø) ⬇️
...rategy/triplea/attachments/UnitTypeComparator.java 35.71% <0%> (-7.15%) 10% <0%> (-1%)
...rc/main/java/games/strategy/net/nio/NioWriter.java 66.94% <0%> (-1.7%) 20% <0%> (ø)
...tegy/triplea/oddsCalculator/ta/OddsCalculator.java 43.39% <0%> (-0.32%) 13% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad6ed80...0752369. Read the comment docs.

README.md Outdated
- Download and install TripleA: http://triplea-game.github.io/download/
# Installing TripleA and Playing

- Download and install TripleA: http://www.triplea-game.org/download/
Copy link
Member

Choose a reason for hiding this comment

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

www.triplea-game.org should be just triplea-game.org

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@RoiEXLab RoiEXLab left a comment

Choose a reason for hiding this comment

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

I believe for this PR @veqryn @ron-murhammer and @DanVanAtta should all approve...

README.md Outdated

Development
===========
- Project documentation, including 'how to get started guides' at: https://github.com/triplea-game/triplea/tree/master/docs/dev
- Broken map list: https://github.com/triplea-game/triplea/wiki/Broken-Maps
- Additional feature request list: https://github.com/triplea-game/triplea/issues?q=label%3A%22ice+box+-+revisit+later%22+is%3Aclosed (list is not to be added to, if picked up, re-open and remove the ice-box label)
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, but the Ice Box label was renamed to ice box - close and revisit later, so this link is broken.
Additionally this link should be hidden behind a nicer name, it really does not look good in the mark down.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. In general, I don't like dropping bare URLs in Markdown, especially ones as busy as the ice box link. How about if I just hide all links in the README behind a label? For example:

Development

Copy link
Member

Choose a reason for hiding this comment

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

👍 For consistency

Copy link
Member Author

Choose a reason for hiding this comment

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

0752369

Thanks for catching the broken ice box link. I tested all the others; not sure why I skipped that one. 🤷‍

Also fix broken ice box issue link.
@ron-murhammer ron-murhammer self-requested a review February 3, 2018 22:45
@DanVanAtta DanVanAtta merged commit d48bff6 into triplea-game:master Feb 9, 2018
@ssoloff ssoloff deleted the issue-2764-upgrade-license-to-gpl-3.0-or-later branch February 9, 2018 03:26
@ssoloff
Copy link
Member Author

ssoloff commented Feb 9, 2018

I believe the license badge in the README will automatically update once this PR is merged.

Confirmed.

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.

None yet

6 participants