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

Adding automatic dependency management #907

Merged
merged 2 commits into from
Dec 19, 2022
Merged

Adding automatic dependency management #907

merged 2 commits into from
Dec 19, 2022

Conversation

DarkWanderer
Copy link
Contributor

There is an open security advisory for time library which chrono is dependent upon: https://github.com/DarkWanderer/DynamicOperations/security/dependabot/4
Having dependabot integration will help to react to such issues in timely manner

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Thanks!

Note that we will not be able to upgrade to time 0.1 in the chrono 0.4.x series due to compatibility constraints but we've already removed it from the main branch.

@esheppa
Copy link
Collaborator

esheppa commented Dec 15, 2022

From what I've seen elsewhere dependabot can often provide a lot of PRs for semver patch updates. I wonder whether we would be better off adding cargo audit and similar to our CI runs instead?

@esheppa
Copy link
Collaborator

esheppa commented Dec 15, 2022

Looks like we already have dependabot security updates enabled for this repo, and from what I can see over at SO this is the only way to get dependabot to do security updates only.

@esheppa esheppa mentioned this pull request Dec 15, 2022
@djc
Copy link
Member

djc commented Dec 15, 2022

I usually enable a Dependabot config like this in most of the projects I maintain. It will send PRs for semver-incompatible releases in our dependencies, which I think is useful, but it also has pretty good support for ignoring new releases (like time 0.3.x). It will not send PRs for semver-compatible releases, so I think this would be good addition.

I also usually add cargo-deny to my CI configurations, which will help specifically with the security aspects. I'll submit a PR for that.

@LingMan
Copy link
Contributor

LingMan commented Dec 15, 2022

From what I've seen Dependabot only sends PRs for semver patch updates if Cargo.lock is checked in. Even then it's pretty easy to disable them like this.

I'd only change the interval to weekly, since checking for updates every single day is way overkill.

@LingMan
Copy link
Contributor

LingMan commented Dec 15, 2022

Oh, and it would be good to add an entry covering fuzz/Cargo.toml as well.

@LingMan
Copy link
Contributor

LingMan commented Dec 15, 2022

Don't worry about the failed lint check. That's a new clippy warning that shows up since Rust 1.66 was released a few hours ago. A fix is included in this PR.

@@ -0,0 +1,14 @@
version: 2
updates:
- package-ecosystem: cargo
Copy link
Contributor

Choose a reason for hiding this comment

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

Forgot the quotes around cargo. Same in the second entry below.

Suggested change
- package-ecosystem: cargo
- package-ecosystem: "cargo"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I didn't forget anything, it works this way. YML spec allows for unquoted string values.

The fact that I listened to suggestions above does not mean that I will spend my time on nitpicks however

Copy link
Contributor

Choose a reason for hiding this comment

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

I only noticed since you did use quotes for github-actions. My apologies if I somehow offended you. Personally I'm always happy if someone points out such inconsistencies in my code.

@djc djc reopened this Dec 19, 2022
@djc djc requested a review from esheppa December 19, 2022 08:54
Copy link
Collaborator

@esheppa esheppa left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @DarkWanderer for the PR and @djc for the clarification

@djc djc merged commit 72bf449 into chronotope:main Dec 19, 2022
@DarkWanderer DarkWanderer deleted the dependabot branch January 7, 2023 13:47
@pitdicker pitdicker mentioned this pull request Jul 7, 2023
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