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 changing versions not keeping the paths #2355

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

syphar
Copy link
Member

@syphar syphar commented Dec 6, 2023

This is a regression from #2113 , fixes #2349

This fix here is mostly copying logic from the get_all_platforms_inner method in this module.

I'm seeing that plenty of logic around version-matching is duplicated between handlers, and will try to find a way to refactor this. Also the URL-Patterns for the target-redirect lead to the handler itself being quite complex, perhaps we could just use GET parameters there. ( also separate IMO).

@syphar syphar requested a review from a team as a code owner December 6, 2023 10:23
@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Dec 6, 2023
@GuillaumeGomez
Copy link
Member

Thanks for the fix. Agreed, a lot of logic is duplicated in a few places, would be nice to centralize it. Also please add a test to prevent this regression from happening again.

@syphar
Copy link
Member Author

syphar commented Dec 6, 2023

( forget it, found the existing tests)

@syphar
Copy link
Member Author

syphar commented Dec 8, 2023

@GuillaumeGomez I added a short test for the partial

@syphar
Copy link
Member Author

syphar commented Dec 8, 2023

Also I just rebased on master and adapted the initial commit based on the changes in #2325

@GuillaumeGomez
Copy link
Member

Looks all good to me, thanks!

@syphar syphar merged commit df4bbb2 into rust-lang:master Dec 8, 2023
10 checks passed
@syphar syphar deleted the fix-release-list-try1 branch December 8, 2023 13:42
@github-actions github-actions bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Dec 8, 2023
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Dec 8, 2023
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.

Changing versions doesn't stay on same item
2 participants