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

Use default bump strategy for dependabot #585

Merged
merged 4 commits into from
Aug 14, 2023

Conversation

PerchunPak
Copy link
Member

@PerchunPak PerchunPak commented Jul 10, 2023

Default strategy works okay on my repositories and currently dependabot doesn't change pyproject.toml at all, I hope this PR will fix this.

Strategy was introduced in #522.

Default strategy works okay on my repositories and
currently dependabot doesn't change `pyproject.toml`
at all, I hope this PR will fix this
@PerchunPak PerchunPak requested a review from ItsDrike as a code owner July 10, 2023 23:16
kevinkjt2000
kevinkjt2000 previously approved these changes Jul 10, 2023
@ItsDrike
Copy link
Member

I thought the reason we used increase-if-necessary was to allow people using mcstatus to use it with older deendencies. I'm fine with using default for development dependencies, but for runtime dependencies, it does make sense to avoid updating pyproject.toml, because it enforces a new minimum version, as opposed to simply allowing it.

poetry.lock is a file that specifies which dependencies we use exactly. However pyproject.toml specifies the dependencies, and the allowed versions that can be installed and will still work without any issues with the rest of our project.

I don't necessarily want to force library users to use the newest possible version, as other libraries that specify the same dependency might do so in a more limiting manner (like for example == specification), potentially making mcstatus incompatible with such libraries.

@ItsDrike ItsDrike added area: dependencies Related to package dependencies type: rewrite Complete or partial rewrite of a part of the codebase labels Jul 11, 2023
@kevinkjt2000
Copy link
Contributor

kevinkjt2000 commented Jul 11, 2023

I only see dev dependency groups being updated. Would this influence the main group of dependencies? Should we look at what dependabot generates and make changes after?

@ItsDrike
Copy link
Member

Dependabot updates all dependencies, both development and main ones. It's just that we don't have very many main dependencies, and so the updates you see most often are for the development deps, which there's a lot of.

@kevinkjt2000
Copy link
Contributor

kevinkjt2000 commented Jul 12, 2023

Does Dependabot allow different configuration overrides? If it does, we could place an override for any explicitly called-out development (or runtime?) dependencies. I'm also okay with ignoring the runtime updates when they pop up in PRs, but one could accidentally slip by since I tend to merge those without much thought if all the checks are passing. Runtime dependency updates would only take effect during the next release, giving us time to revert an accidental update. Should we experiment with this new strategy and then react based on Dependabot's new behavior?

Edit: Would this work for us? Or would we still need multiple "pip" configurations to allow these to update-when-necessary?

    ignore:
      - dependency-name: "asyncio-dgram"
        update-types: ["version-update:semver-minor"]
      - dependency-name: "dnspython"
        update-types: ["version-update:semver-minor"]

@ItsDrike
Copy link
Member

ItsDrike commented Jul 12, 2023

We should be able to do even better than that, we can just have 2 pip configurations use dependency-type: development and dependency-type: production. Dependabot should be smart enough to know which deps are which, no need to specify them manually. For production, the strategy should stay increase-if-necessary, and for development, we can use the default one.

@kevinkjt2000 kevinkjt2000 dismissed their stale review July 12, 2023 23:48

In favor of ItsDrike suggestions

@PerchunPak
Copy link
Member Author

I thought the reason we used increase-if-necessary

The reason was described in #522, it was done to workaround annoying dependabot's bug, which I can't reproduce now.

for runtime dependencies, it does make sense to avoid updating pyproject.toml, because it enforces a new minimum version, as opposed to simply allowing it.

We should then use >=1.0.0,<1.3.0 (which allows 1.0, 1.1, 1.2 but not 1.3 (because it may be unreleased or was untested on the moment of our last release)). If you want to allow all next versions, let's use >=. If you want to allow all next minor versions, let's use tilde on major version (e.g. ~1 is equal to >=1.0.0,<2.0.0, which will accept any minor version of 1.x.x but not 2.0.0 or newer).

I just find it confusing, when dependency's version is different in pyproject.toml and poetry.lock. We should clearly show that we support those versions, if we really want so.

I don't necessarily want to force library users to use the newest possible version

This is why caret dependency specification exist in poetry. It just allows any semver-compatible version.

Edit: Would this work for us? Or would we still need multiple "pip" configurations to allow these to update-when-necessary?

It should work, I use something same in my other projects.

@PerchunPak
Copy link
Member Author

(can't re-request review from @ItsDrike)

@PerchunPak PerchunPak merged commit cd8bf38 into master Aug 14, 2023
@PerchunPak PerchunPak deleted the dependabot-default-strategy branch August 14, 2023 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dependencies Related to package dependencies type: rewrite Complete or partial rewrite of a part of the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants