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

Add dependabot for dependency upgrades #763

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

clintonsteiner
Copy link
Collaborator

@clintonsteiner clintonsteiner self-assigned this Oct 21, 2024
@akaihola akaihola self-requested a review October 22, 2024 19:58
@akaihola akaihola added enhancement New feature or request CI labels Oct 22, 2024
@akaihola akaihola added this to the Darker 3.1.0 milestone Oct 22, 2024
@akaihola
Copy link
Owner

Thanks @clintonsteiner!

So does dependabot work fine with packages which use setuptools and setup.cfg? I remember participating in the discussion of dependabot/dependabot-core#2133, but it might be that the issue concerns only the depenedency graph feature on GitHub.

Copy link
Owner

@akaihola akaihola left a comment

Choose a reason for hiding this comment

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

Can we somehow test this before merging?

Also, a change log entry would be nice.

@clintonsteiner
Copy link
Collaborator Author

https://github.com/clintonsteiner/darker/actions/runs/11605270324/job/32315372582
Testing in fork now appears to work, must enable in code security though
image

@clintonsteiner
Copy link
Collaborator Author

#767
I'd like to get rid of the constraints-oldest and move to pyproject.toml

I was unable to make dependabot acknowledge both files and seems a pain to manage both

@@ -60,7 +60,7 @@ color =
Pygments>=2.4.0
test =
# NOTE: remember to keep `constraints-oldest.txt` in sync with these
black>=22.3.0
black>=24.1.0
Copy link
Owner

Choose a reason for hiding this comment

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

What, did dependabot claim there are vulnerabilities in older versions of Black?

Copy link
Owner

Choose a reason for hiding this comment

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

I do see a CVE security vulnerability in Black 24.3.0 though. But I couldn't find anything before 24.1.0 which would have been fixed in that version.

I guess it makes sense to drop support for Black <24.3.0, but on the other hand I wouldn't like to break users' workflows if they for any reason choose to pin to a selected Black version. And if the code they are formatting is all internal and trusted, I think that's a valid decision.

What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

I wonder how dependabot will react to dependencies in extras – Black will be moved there in #759.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It tried to update the constraints-oldest.txt file but left the setup.cfg alone

Think we need to have dependencies defined only in one spot for dependabot

@akaihola
Copy link
Owner

akaihola commented Nov 7, 2024

#767
I'd like to get rid of the constraints-oldest and move to pyproject.toml

I was unable to make dependabot acknowledge both files and seems a pain to manage both

I noticed you closed #767 – did you find a work-around?

I wanted to move away from setup.cfg as well, and uv nowadays provides --resolution=lowest, equivalent to what I've used constraints-oldest.txt for.

But: Due to missing raw HTML support on PyPI, we still depend on setup.py to strip the contributor table from README.rst in the distribution packages. This is the only reason we're still using setup.py and setup.cfg. I've been looking into different ways of solving this (e.g. configuring PyPI or GitHub to use a different file for the project front page), but haven't found an elegant solution yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

2 participants