Skip to content

Download cleanup - #4531

Merged
djc merged 7 commits into
mainfrom
download-cleanup
Oct 16, 2025
Merged

djc merged 7 commits into
mainfrom
download-cleanup

Conversation

@djc

@djc djc commented Oct 13, 2025 •

Copy link
Copy Markdown
Contributor

Followup from:

Sorry @FranciscoTGouveia, here are some more changes which IMO make things a bunch cleaner. I haven't done any testing yet (would be great if you want to do some of that) but I think the changes here are relatively straightforward?

@djc
djc requested review from ChrisDenton and rami3l October 13, 2025 10:49
@djc
djc force-pushed the download-cleanup branch from 3664131 to 360dd06 Compare October 13, 2025 11:40

@rami3l rami3l left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM modulo some questions :)

Comment thread src/utils/mod.rs Outdated
Comment thread src/dist/download.rs Outdated
Comment thread src/cli/self_update.rs
@rami3l

rami3l commented Oct 13, 2025

Copy link
Copy Markdown
Member

Please note that since this PR is related to more subtle changes in the user behavior that is not fully covered by the test suite, more attention should be paid to ensure the necessary preservation of original semantics.

@djc
djc force-pushed the download-cleanup branch from 360dd06 to b96b51e Compare October 13, 2025 14:37
@djc
djc force-pushed the download-cleanup branch from b96b51e to 9364884 Compare October 14, 2025 08:54
@djc
djc force-pushed the download-cleanup branch from 9364884 to cc732d2 Compare October 14, 2025 11:07
@djc

djc commented Oct 16, 2025

Copy link
Copy Markdown
Contributor Author

Please note that since this PR is related to more subtle changes in the user behavior that is not fully covered by the test suite, more attention should be paid to ensure the necessary preservation of original semantics.

Do you think there's anything left to do here, or is it okay to merge this?

@rami3l

rami3l commented Oct 16, 2025

Copy link
Copy Markdown
Member

@djc I think it should be okay to merge this patch now that the concerns have been addressed. If there are further problems we can fix them separately later.

@djc
djc added this pull request to the merge queue Oct 16, 2025
Merged via the queue into main with commit c59cd9c Oct 16, 2025
29 checks passed
@djc
djc deleted the download-cleanup branch October 16, 2025 09:15
@djc djc mentioned this pull request Oct 16, 2025
5 tasks done
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.

3 participants