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

Dependency-Review defaults update #53

Merged
merged 3 commits into from
Dec 15, 2022
Merged

Dependency-Review defaults update #53

merged 3 commits into from
Dec 15, 2022

Conversation

jentfoo
Copy link
Contributor

@jentfoo jentfoo commented Dec 9, 2022

Update dependency-review to also by default include the master branch. In addition the minimal severity for failing was raised to moderate.

I think this will allow most projects to accept this configuration without modification.

Update dependency-review to also by default include the `master` branch.  In addition the minimal severity for failing was raised to moderate.
@jentfoo jentfoo requested review from reedloden, adaadb6 and a team December 9, 2022 22:59
@@ -12,6 +12,7 @@ on:
pull_request:
branches:
- main
- master
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be handled on the calling repo, not this repo. Look at how this is called on teleport repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I was trying to see if we could reduce how many repo's needed the branch definition. If we want it to be defined on the calling repo always should I remove the branches entirely?

@@ -29,4 +30,5 @@ jobs:
- name: 'Dependency Review'
uses: actions/dependency-review-action@11310527b429536e263dc6cc47873e608189ba21
with:
fail-on-severity: moderate
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have cases where we have low severity issues causing problems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As best I can tell the teleport repo is the only repo that has this rolled out so far. So no, but I was concerned it may cause some noise as we roll it out. If you want to start stricter I am fine with it, I just wondered if it had been considered.

@reedloden reedloden merged commit e098369 into main Dec 15, 2022
@reedloden reedloden deleted the jent/dependency-review branch December 15, 2022 00:48
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.

4 participants