Skip to content

Add a lockfile#1337

Draft
clux wants to merge 2 commits into
mainfrom
lockfile-add
Draft

Add a lockfile#1337
clux wants to merge 2 commits into
mainfrom
lockfile-add

Conversation

@clux
Copy link
Copy Markdown
Member

@clux clux commented Nov 8, 2023

From a blank build today. I personally think this makes sense.

Reasons;

  • basically https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html
  • frequently we get cargo deny issues pulled in from under us which we have to explain away
  • contributor confusion; why does this not build? we don't want them to be the first to notice a breaking build, that's CIs job
  • we can setup dependabot to auto-merge non-breaking changes that pass CI

Downsides:

  • we still have to manually fix up most cargo deny issues
  • we will get a lot more dependency prs, but if most are auto-merged then it should be ok? they don't show up in the github releases because of our changelog configuration

AFAIU non-breaking builds do not need to get pins updated from Cargo.toml so this should mostly be a sanity thing for CI and contributors (rather than forcing everyone to bump the dependencies of us).

Have setup automatic dependency merging in all other kube repos, but those are binaries so it's less scary / controversial. My experience with these have been very positive however, so feel we should probably do this here also.

From a blank build today.

Reasons;

- basically https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html
- frequently we get cargo deny issues pulled in from under us which we have to explain away
- contributor confusion; why does this not build? we don't want them to be the first to notice a breaking build, that's CIs job
- we can setup dependabot to auto-merge non-breaking changes that pass CI

Downsides:

- we still have to manually fix up most cargo deny issues
- we might get a lot more PRs, but if most are auto-merged that's ok

AFAIU non-breaking builds do not need to get pins updated from `Cargo.toml` so this should mostly be a sanity thing for CI and contributors (rather than forcing everyone to bump the dependencies of us).

Signed-off-by: clux <sszynrae@gmail.com>
@clux clux added the changelog-exclude changelog excluded prs label Nov 8, 2023
@clux clux added this to the 0.88.0 milestone Nov 8, 2023
Signed-off-by: clux <sszynrae@gmail.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 8, 2023

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.1%. Comparing base (5813ad0) to head (01dec7f).
⚠️ Report is 571 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1337   +/-   ##
=====================================
  Coverage   72.1%   72.1%           
=====================================
  Files         75      75           
  Lines       6377    6377           
=====================================
  Hits        4597    4597           
  Misses      1780    1780           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Dav1dde
Copy link
Copy Markdown
Member

Dav1dde commented Nov 8, 2023

Re: dependabot, I feel like there should be just a weekly lockfile maintenance which bumps the lockfile to the max dependency versions and not for each dependency individually

@clux
Copy link
Copy Markdown
Member Author

clux commented Nov 8, 2023

We can do that I believe; weekly interval on dependabot with a grouping on "*"

@clux clux removed this from the 0.88.0 milestone Nov 8, 2023
@clux clux added the question Direction unclear; possibly a bug, possibly could be improved. label Nov 8, 2023
@nightkr
Copy link
Copy Markdown
Member

nightkr commented Dec 1, 2023

Very 👍 on this from me.

@clux
Copy link
Copy Markdown
Member Author

clux commented Dec 4, 2023

I think this is nice personally also, but we could also get a decent approximation of safety with a daily build.

so a couple of things that would be good to get feelings on;

  • lockfile safety vs. cron'd check of latest (cargo update + build)
  • bump frequency / dependabot grouping setup / automerging (if using lockfile)
  • MSRV implications (if we tail latest with a lockfile, then maybe we change msrv test to use -Zminimal-versions builds? - maybe we should do that anyway)

Feel free to leave comments here, but have also added it as an agenda item for tomorrow's meeting :-)

@nightkr
Copy link
Copy Markdown
Member

nightkr commented Dec 4, 2023

Why not both?

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

Labels

changelog-exclude changelog excluded prs question Direction unclear; possibly a bug, possibly could be improved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants