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

Simplify license logic #807

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Conversation

amvanbaren
Copy link
Contributor

@kineticsquid, can you run this PR on Gitpod and check the new license logic?

@amvanbaren amvanbaren self-assigned this Sep 5, 2023
@kineticsquid
Copy link

@amvanbaren Cetainly. I have a workspace up now based a a fork of your repo using this branch. I'll create some samples to test the different scenarios.

@kineticsquid
Copy link

kineticsquid commented Sep 21, 2023

@amvanbaren Here are the scenarios I tried and my results. 'Prod' is current behavior. 'PR 807' are your updates. I have a fork of your repo with the test extension versions I used: https://github.com/kineticsquid/openvsx/tree/feature/issue-777. We are no longer doing the license detection, this is good. The scenarios we may need work on are 1.0.0 and 3.0.0.

  • 1.0.0 No license in package.json and no license file
    - Prod: Error on publish due to lack of license
    - PR 807: Publishing does not fail. Unlicensed displayed. Was this behavior intentional? I think we want the original behavior and fail the publish. Our terms say you must have a license.

  • 2.0.0 No license in package.json and license.txt file is provided:
    - Prod: Extension details page shows Provided license and links to provided license file, license.txt.
    - PR 807: Same - OK

  • 3.0.0 No license in package.json and license.md file is provided:
    - Prod: Extension details page shows Provided license and links to provided license file,license.md.
    - PR 807: Link shows http:\\...file\license.md but file downloaded is f.txt. Contents or f.txt are correct. This appears to be an error.

  • 4.0.0 No license in package.json and license file is provided:
    - Prod: Extension details page shows Provided license and links to provided license file, license.txt.
    - PR 807: same - OK

  • 5.0.0 No license in package.json and LICENSE file is provided
    - Prod: Extension details page shows Provided license and links to provided license file, LICENSE.txt.
    - PR 807: Same - OK

  • 6.0.0 No license in package.json and license.txt file is provided, that contains verbatim MIT license.
    - Prod: MIT displayed and links to license.txt. This is the behavior we wanted to change.
    - PR 807: Extension details page shows Provided license and links to provided license file, license.txt. - OK

  • 7.0.0 My special license specified for license in package.json and no license file:
    - Prod: My special license is displayed with a link to https://spdx.org/licenses/My%20special%20license.html which results in a 404. This behavior we wanted to change
    - PR 807: My special license is displayed with no link. - OK

  • 8.0.0 My special license specified in package.json and license file is provided:
    - Prod: My special license text is displayed with a link to the license file
    - PR 807: same - OK

  • 9.0.0 My special license specified in package.json and license.txt file is provided, that contains verbatim MIT license.
    - Prod: My special license text is displayed with a link to the license file. Does not try to determine what type of license has been provided
    - PR 807: Same - OK

@amvanbaren
Copy link
Contributor Author

Publishing does not fail. Unlicensed displayed. Was this behavior intentional? I think we want the original behavior and fail the publish. Our terms say you must have a license.

ovsx.publishing.require-license isn't set for development. On staging and production require-license is set to true.

Link shows http:\...file\license.md but file downloaded is f.txt. Contents or f.txt are correct. This appears to be an error.

File contents are stored in the database by default. On staging and production Azure Blob Storage is used instead.

@amvanbaren amvanbaren merged commit 3e9e023 into eclipse:master Oct 5, 2023
2 checks passed
@kineticsquid
Copy link

Understood, thanks.

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

Successfully merging this pull request may close these issues.

2 participants