-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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(dotnet): add license support for NuGet #5217
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very nice! I left small comments.
if license := pkg.Metadata.License; license.Type == "expression" && license.Text != "" { | ||
return []string{license.Text}, nil | ||
} | ||
|
||
return nil, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I prefer the happy path getting aligned to the left edge.
https://medium.com/@matryer/line-of-sight-in-code-186dd7cdea88
if license := pkg.Metadata.License; license.Type == "expression" && license.Text != "" { | |
return []string{license.Text}, nil | |
} | |
return nil, nil | |
if license := pkg.Metadata.License; license.Type != "expression" || license.Text == "" { | |
return nil, nil | |
} | |
return []string{pkg.Metadata.License}, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in 1d0fed1
if err != nil { | ||
if !errors.Is(err, fs.ErrNotExist) { | ||
return xerrors.Errorf("license find error: %w", err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: We might want to aggregate the statements.
if err != nil { | |
if !errors.Is(err, fs.ErrNotExist) { | |
return xerrors.Errorf("license find error: %w", err) | |
} | |
} | |
if err != nil && !errors.Is(err, fs.ErrNotExist) { | |
return xerrors.Errorf("license find error: %w", err) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! Thanks! Updated in 1d0fed1.
@knqyf263 looks like this PR has not been added to merge queue. |
CLA assistant didn't work. I opened the following link to trigger it. |
Description
packages.config
andpackages.lock.json
files don't have information about the licenses used.Add support of *.nuspec files from global packages folder.
notes:
NUGET_PACKAGES
environment variable are supported.licenseUrl
is not supported, because this field has been deprecated.expression
only supportsExample:
Related issues
Checklist