search

georust / geo

Geospatial primitives and algorithms for Rust
https://crates.io/crates/geo
Other
1.52k stars 196 forks source link

Should we enable dependabot? #863

Open michaelkirk opened 2 years ago

michaelkirk commented 2 years ago

Dependabot is a github bot that will automatically open PR's with dependency updates.

The PR's look like this: https://github.com/urschrei/polylabel-rs/pull/23

Until now we've been using the very-reliable and polite dependa-martin (see #849, #857, #858, #859), and really I have no complaints, but it doesn't seem super scalable.

What would people think about enabling dependabot for the geo/geo-types repo?

martinfrances107 commented 2 years ago

ha-ha thank you... I am blushing slightly, very un-robot-like of me.

Dependabot sounds like a good idea.

I am reading this config guide

https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuring-dependabot-version-updates

and trying to set a series of discussion points ... which will then get encoided in the various config settings. One item of complexity I see is that our extensive use of work spaces ( which is a good thing) means that we will have a very detailed dependabot.yaml file.

When I use this rustcrate to auto generate a sample dedendabot.yaml files. I see that a complex skeleton is already constructed for us. https://github.com/andreimoustache/rust-dependabot-generator

Can I starting the discussion off by suggesting that we mandate a weekly check for most workspaces but set the more sensitive geo-types to be on a 6 month upgrade warning cycle?

cat .github/dependabot.yaml


---
version: 2
updates:
  - package-ecosystem: cargo
    directory: geo
    schedule:
      interval: weekly
  - package-ecosystem: cargo
    directory: geo/fuzz
    schedule:
      interval: weekly
  - package-ecosystem: cargo
    directory: geo-bool-ops-benches
    schedule:
      interval: weekly
  - package-ecosystem: cargo
    directory: jts-test-runner
    schedule:
      interval: weekly
  - package-ecosystem: cargo
    directory: geo-postgis
    schedule:
      interval: weekly
  - package-ecosystem: cargo
    directory: /
    schedule:
      interval: weekly
  - package-ecosystem: cargo
    directory: geo-test-fixtures
    schedule:
      interval: weekly
  - package-ecosystem: cargo
    directory: geo-types
    schedule:
      interval: weekly
lnicola commented 2 years ago

I think periodic dependency bumps are less (but still) important for library crates, and the PRs can produce a lot of noise when they happen across 30 crates at a time.

And I just noticed a strange behavior in https://github.com/rust-lang/vscode-rust/pull/936#issuecomment-1166276031. An unmerged PR to bump a package from 2.6.1 to 3.1.1 was automatically closed as obsoleted by a PR bumping it from 2.6.1 to 2.6.7, which is probably not ideal.

One item of complexity I see is that our extensive use of work spaces ( which is a good thing) means that we will have a very detailed dependabot.yaml file.

Ouch, is that really necessary?

martinfrances107 commented 2 years ago

I just want to flesh out various arguments that I hear in this regard. I will try and speak neutrally with settling on one side or the other.

So there are people in say banking or other security minded fields. where A java-script is seen as a fundamental security risk... but if java-script are to be tolerated then audits are conducted monthly and only small set of whitelisted packages is maintained/permitted, Here Dependabot is standard.

I appreciate there are also project where even the financial burden of a 2-year audit is too much.

I saw an interview with Linus Torvalds recently where he said he is having to adjust his perceptions because the manufacturers of IOT devices for street furniture... are forming a alliance where they maintain a release of Linux with a 25 year support life cycle. No additional features, not even support for well documented bugs. Only changes for once in a lifetime catastrophic events, such as the "Spectre" or "Meltdown" vulnerability becoming public. That is a difficult problem space as the alliance was only ingesting images for consideration that were at least 5 years old - and they weren't going to change their policy.

The standard big project, well funded, response is the have a to Fast Moving branch, a LTS branch, and various legacy branches, for key constituents.

I appreciate the burdens on project maintainers, and don't want appear careless with other peoples time.

So my only real opinion, is to say OMG that is expensive and often unworkable.

Ouch, is that really necessary?

@lnicola May I ask a question? What is your perspective, What is the period between maintenance on your projects that involve georust/geo

lnicola commented 2 years ago

Ouch, is that really necessary?

What I meant is that I'd expect dependabot to understand a workspace lockfile and be able to work without any manually defined (or automatically defined) configuration. And maybe it already does?

What is the period between maintenance on your projects that involve georust/geo

I'm new around here, but in general I'm of the opinion that dependency bumps are easier when done often. So I think dependabot is fine as long as:

And it's not like we're making a permanent decision, we can try it and see what happens.

urschrei commented 2 years ago

it doesn't file PRs for semver-compatible bumps (e.g. updating to 1.26.2 when Cargo.toml says 1.26.0)

I think it does this, alas.

it doesn't do stupid things like closing minor bump PRs in favour of a patch bump (like in my example above)

I'm not sure what its mechanism is here – we should def clarify that.

michaelkirk commented 2 years ago

it doesn't file PRs for semver-compatible bumps (e.g. updating to 1.26.2 when Cargo.toml says 1.26.0) more so in a library crate, where the exact versions are the user's choice

I've never used it, but maybe relevant: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#versioning-strategy

In particular these look like interesting options:

widen: Relax the version requirement to include both the new and old version, when possible. increase-if-necessary: Increase the version requirement only when required by the new version.

💩 I realize these versioning strategies are not available for rust/cargo though. =(

I think this issue is related: https://github.com/dependabot/dependabot-core/issues/4009

tobymurray commented 2 years ago

With the disclaimer that I'm not involved in this project at all (except as a consumer), I've personally been responsible for enabling Dependabot at my day job. My experience heavily favored:

And it's not like we're making a permanent decision, we can try it and see what happens.

I'd suggest starting with a low (e.g. 1) value for open-pull-requests-limit for each, and if you're really nervous, enable it for a single workspace, but other than that just go for it. As comfort and familiarity grows, the limits can be increased (or not). It's extremely low risk and the practical experience grounded discussions so we didn't end up chasing a bunch of "what if" scenarios.