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

fix: update ruff manifest #501

Merged
merged 4 commits into from
Jul 24, 2024
Merged

fix: update ruff manifest #501

merged 4 commits into from
Jul 24, 2024

Conversation

gracevivi523
Copy link
Contributor

@gracevivi523 gracevivi523 commented Jul 22, 2024

ruff seems to have changed the download link, this PR to update the link and resume auto-update for ruff again.

reference from https://github.com/astral-sh/ruff/releases

tested locally by installing 3 different version from 3 different categories as they have slightly different source url as well as their packaging approach has changed slightly.
image

Copy link
Collaborator

@lyonlai lyonlai left a comment

Choose a reason for hiding this comment

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

To do so you will need to update all the sha256sums in the manifest file.

you will need to remove the existing entries and do hermit manifest add-digests {your package manifest}

@gracevivi523
Copy link
Contributor Author

gracevivi523 commented Jul 22, 2024

To do so you will need to update all the sha256sums in the manifest file.

you will need to remove the existing entries and do hermit manifest add-digests {your package manifest}

Thank you @lyonlai.

Actually I think the links works for older version til 0.1.7, 0.1.8+ requires the the new link, is there a way that can cover both?

@gracevivi523
Copy link
Contributor Author

or maybe we do not need to worry about the old versions <=0.1.7, but will the change in this PR break existing repositories that are using ruff <=0.1.7 ?

@alecthomas
Copy link
Collaborator

Older versions need to remain, but you should be able to do exactly what @lyonlai suggested and everything "should" work barring weird issues.

@damienrj
Copy link
Contributor

Older versions need to remain, but you should be able to do exactly what @lyonlai suggested and everything "should" work barring weird issues.

It looks like older versions should work since they have migrated them over as well. https://github.com/astral-sh/ruff/releases/tag/v0.1.7 For example.

@gracevivi523
Copy link
Contributor Author

gracevivi523 commented Jul 22, 2024

It looks like older versions should work since they have migrated them over as well. https://github.com/astral-sh/ruff/releases/tag/v0.1.7 For example.

There is a slight difference, the older link has v before the version value v0.1.7, while the new link is not, it's 0.5.4 without v.

@damienrj
Copy link
Contributor

It looks like older versions should work since they have migrated them over as well. https://github.com/astral-sh/ruff/releases/tag/v0.1.7 For example.

There is a slight difference, the older link has v before the version value v0.1.7, while the new link is not, it's 0.5.4 without v.

Yeah, the url change which you caught. I just meant that the binaries are still there.

@gracevivi523
Copy link
Contributor Author

PR is ready for 2nd round of review cc @lyonlai @alecthomas

@gracevivi523 gracevivi523 changed the title update source for ruff fix: update ruff Jul 23, 2024
@gracevivi523 gracevivi523 changed the title fix: update ruff fix: update ruff manifest Jul 23, 2024
ruff.hcl Outdated Show resolved Hide resolved
ruff.hcl Outdated Show resolved Hide resolved
Copy link
Collaborator

@lyonlai lyonlai left a comment

Choose a reason for hiding this comment

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

I suspect the unknown in the linux url is still messy and might be clean up for a bit in the future. But that's a problem for tmr

@lyonlai lyonlai merged commit 566e75a into cashapp:master Jul 24, 2024
2 checks passed
@gracevivi523 gracevivi523 deleted the patch-1 branch July 24, 2024 04:48
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