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

feat(python)!: default license to Apache-2.0 if unset #3395

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

garysassano
Copy link
Contributor

@garysassano garysassano commented Feb 25, 2024

Adding a default Apache-2.0 license to PythonProject to align it with NodeProject.

BREAKING CHANGE: The PythonProject now defaults to an Apache-2.0 license. To use a different license, you must specify the license property in your project configuration.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link

codecov-commenter commented Feb 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.34%. Comparing base (f41cf3c) to head (3e4f0ea).
Report is 5 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3395      +/-   ##
==========================================
+ Coverage   96.32%   96.34%   +0.02%     
==========================================
  Files         191      192       +1     
  Lines       37407    37568     +161     
  Branches     3489     3509      +20     
==========================================
+ Hits        36032    36195     +163     
+ Misses       1375     1373       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

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

Do we really not have any tests for this? 😱

Also this looks like it will be a breaking change.

Copy link
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

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

Sorry, this is going to be super weird. Because license is defined here: https://github.com/projen/projen/blob/main/src/python/python-packaging.ts#L48

We need to add a @default but then this also needs to change for all usages of PythonPackagingOptions, not just poetry.

@mrgrain mrgrain changed the title chore(python): add default Apache-2.0 license to PythonProject with Poetry or Setuptools feat(python)!: default license to Apache-2.0 if unset Feb 26, 2024
Copy link
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

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

Code looks good. Will holding this back to group with at least one other Breaking Change.

I'm also still wondering if we have a better transition path here. This feels high risk to unintentionally license a bunch of packages under Apache-2.0

@mrgrain
Copy link
Contributor

mrgrain commented Mar 25, 2024

@garysassano Been thinking more about this. We will at least have to add a warning about undefined license defaulting to Apache2.0. The warning can be removed with GA (make sure to add a @todo with respective comment).

Copy link
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

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

We need a warning for this.

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

3 participants