Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: warn users about missing LICENSE#628

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

Andrewnt219
Copy link
Contributor

@Andrewnt219Andrewnt219 commentedOct 8, 2021•
edited
Loading

Motivation

Fixes#603

When the code fails to automatically detect a LICENSE file, it now warns the users if they want to continue.

Test Plan

Runvsce package without aLICENSE should generate a warning message.

# No LICENSE.# No 'SEE LICENSE IN term.md'PS C:\helloworld> vsce packageExecuting prepublish script'npm run vscode:prepublish'...> helloworld@0.0.1 vscode:prepublish> npm run compile> helloworld@0.0.1 compile> tsc -p ./ WARNING LICENSE.md or LICENSE.txt not foundDo you want to continue? [Y/N]# No LICENSE.# Has 'SEE LICENSE IN term.md'PS C:\helloworld> vsce packageExecuting prepublish script'npm run vscode:prepublish'...> helloworld@0.0.1 vscode:prepublish> npm run compile> helloworld@0.0.1 compile> tsc -p ./ WARNING  term.md not foundDo you want to continue? [Y/N]# Has LICENSE.# Has 'SEE LICENSE IN term.md'PS C:\helloworld> vsce packageExecuting prepublish script'npm run vscode:prepublish'...> helloworld@0.0.1 vscode:prepublish> npm run compile> helloworld@0.0.1 compile> tsc -p ./ WARNING  term.md not foundDo you want to continue? [Y/N]

Copy link
Member

@joaomorenojoaomoreno left a comment

Choose a reason for hiding this comment

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

I have read through the README, but I don't know how I can set up dev environment to reporuce the case.

Yeah, the current steps seems to be broken. I've fixed those inmain.

@joaomorenojoaomoreno self-assigned thisOct 11, 2021
@Andrewnt219Andrewnt219 marked this pull request as draftOctober 12, 2021 03:54
@Andrewnt219Andrewnt219 marked this pull request as ready for reviewOctober 12, 2021 03:57
@Andrewnt219
Copy link
ContributorAuthor

Andrewnt219 commentedOct 12, 2021•
edited
Loading

@joaomoreno I have resolved those issues you mentioned.

Just a bit uncertain about the LICENSE name. Do you want me to extract that matching logic into a private method ofLicenseProcessor, and use that in both theconstruct andonEnd? Or just keep it that way?

@joaomoreno
Copy link
Member

Yeah I would store that into a privateexpectedLicenseName: string property, in the ctor, so we don't have to compute it again.

@Andrewnt219
Copy link
ContributorAuthor

Done. You can refer to theTest Plan for sample console output.

@Andrewnt219
Copy link
ContributorAuthor

Andrewnt219 commentedOct 12, 2021•
edited
Loading

Somehow, the last commit removed some brackets from the methodonFile above it, which made some tests failed. I fixed it.

Co-authored-by: João Moreno <mail@joaomoreno.com>
@Andrewnt219
Copy link
ContributorAuthor

Andrewnt219 commentedOct 12, 2021•
edited
Loading

ah, it is because of the dot inLICENSE.md or LICENSE.txt. I'll revert the regex of default back to the original.

Fixed test [toVsixManifest] should automatically detect license files:
Copy link
Member

@joaomorenojoaomoreno left a comment

Choose a reason for hiding this comment

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

Added a few more commits on top.

@joaomorenojoaomoreno merged commit588ad79 intomicrosoft:mainOct 13, 2021
@joaomoreno
Copy link
Member

Thanks!

@joaomorenojoaomoreno added this to theOctober 2021 milestoneOct 13, 2021
@Andrewnt219
Copy link
ContributorAuthor

@joaomoreno No problem. I'd also be really happy if you can label this PR ashacktoberfest-accepted 😀.

joaomoreno reacted with thumbs up emojijoaomoreno reacted with hooray emojijoaomoreno reacted with heart emojijoaomoreno reacted with rocket emoji

@lnicola
Copy link

lnicola commentedDec 3, 2021•
edited
Loading

I'm not sure how this is supposed to work, but if the license is already defined as a SPDX string inpackage.json, the (previous) code still looked for a file on disk. Is that intentional?

@joaomoreno
Copy link
Member

joaomoreno commentedDec 6, 2021•
edited
Loading

@lnicola The SPDX string only changes the licence file name to look for. It will always look for a license file.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@joaomorenojoaomorenojoaomoreno approved these changes

Assignees

@joaomorenojoaomoreno

Projects
None yet
Milestone
October 2021
Development

Successfully merging this pull request may close these issues.

Prevent / Warn about missing LICENSE
3 participants
@Andrewnt219@joaomoreno@lnicola
[8]ページ先頭
©2009-2025 Movatter.jp