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: added NuGet and npm packages licenses checks on PR build validation #172347 #26

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

Conversation

AlicePayneAudacia
Copy link
Collaborator

Code ready for review

  • ✔ Code compiles, no debugging/console statements or commented out code left in

YAML pipeline syntax validated

  • ✔ YAML syntax is valid and pipeline runs successfully

README or other documentation updated

  • ✔ Documentation updated to reflect the changes made

Added/updated logging

  • ✔ Most/all changed code contains appropriate pipeline logging

Pipeline triggers verified

  • ❎ No changes to pipeline triggers

Secrets and variables managed

  • ❎ No changes to secrets or variables

Dependency licenses

  • ❎ No new/upgraded dependencies

Pipeline steps documented

  • ✔ All new/modified pipeline steps are documented

Security vulnerabilities checked

  • ✔ No new security vulnerabilities introduced/external dependencies are secure and up-to-date

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if these 3 files should be in the build folder? Maybe like the dependency check steps they could live in /src/security/license-checks/?

Not sure they are strictly security though 😆 happy to see what anyone else thinks.

inputs:
scriptType: inlineScript
inlineScript: |
$licenses = @("MIT","Apache-2.0","BSD-3-Clause","Apache-2.0 OR MPL-2.0","MS-EULA","X11","Unlicense","0BSD","BSD-2-Clause")
Copy link
Contributor

Choose a reason for hiding this comment

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

There definitely doesn't need to be any project specific overrides of any of these configs? Or at least not in v1?

@@ -69,6 +69,10 @@ jobs:
--configuration Release
--no-restore

- template: /src/build/common/steps/license-checks.steps.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to happen after the build? It can't go up at the top with the dependency check? Feels a bit random here between Build and Test

steps:
- script: |
dotnet new tool-manifest
dotnet tool install dotnet-project-licenses
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

2 participants