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 svn and bazaar failing to checkout in verbose mode #11051

Merged
merged 4 commits into from
Apr 19, 2024

Conversation

devsagul
Copy link
Contributor

@devsagul devsagul commented Apr 20, 2022

Closes: #11050

@devsagul devsagul force-pushed the fix/svn-fails-with-verbose branch 2 times, most recently from 0739542 to 4acbd04 Compare April 20, 2022 18:14
@devsagul
Copy link
Contributor Author

There is a a failing test: is it a flap? Should I somehow re-run it, or it can be related to the fact that timeout was not processed properly before and test failed because verbose checkouts failed and now I should fix it somehow?

@uranusjr
Copy link
Member

Yeah that does nto seem related. And it only fails on one Windows machine. Don’t worry about it (for this PR), that should be dealt separately.

@github-actions github-actions bot added the needs rebase or merge PR has conflicts with current master label Oct 7, 2022
@dfn-certling
Copy link

Is there something missing here? The fix looks straight forward?

@devsagul
Copy link
Contributor Author

Is there something missing here? The fix looks straight forward?

I guess it should be planned for including into a release by a maintainer and than merged (it was never stated clearly). I will update the PR.

@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Apr 19, 2024
@devsagul devsagul marked this pull request as draft April 19, 2024 12:31
@devsagul devsagul marked this pull request as ready for review April 19, 2024 12:57
@devsagul
Copy link
Contributor Author

Sorry for the mess, haven't been using git for a while

@uranusjr is there something I have to do in order to merge this fix?

@pfmoore
Copy link
Member

pfmoore commented Apr 19, 2024

Thanks for the fix!

@pfmoore
Copy link
Member

pfmoore commented Apr 19, 2024

Hang on, I thought I merged this? When I approved, there were separate tests for verbose and quiet modes, but these have gone now. Why did you remove them? For that matter, why are you still making changes after the PR was approved?

@devsagul
Copy link
Contributor Author

Hang on, I thought I merged this? When I approved, there were separate tests for verbose and quiet modes, but these have gone now. Why did you remove them? For that matter, why are you still making changes after the PR was approved?

Yeah, sorry, I just rebased that, it's better for me to leave it as is if you are going to merge it right away

@devsagul
Copy link
Contributor Author

When I approved, there were separate tests for verbose and quiet modes, but these have gone now. Why did you remove them?

Didn't catch that, the diff is literally the same

@pfmoore pfmoore merged commit 6858c92 into pypa:main Apr 19, 2024
24 checks passed
@pfmoore
Copy link
Member

pfmoore commented Apr 19, 2024

I think what I did was hit "Comment" rather than "Confirm merge". My bad. Sorry for the confusion. Also, while we don't have a policy on force pushes vs extra commits, I find it can be a lot harder to keep track of what's going on with force pushes. Maybe that's why I lost track of the verbose/quiet tests? Not sure. It's not a huge deal wither way.

Anyway, as I said, thanks for the contribution.

@devsagul
Copy link
Contributor Author

devsagul commented Apr 19, 2024

On that note, you've merged without squashing which is a bit unexpected tbh (and now there is my mess of four commits in the main branch; not all of them are valid), maybe something like a policy mentioned here would be nice.

I mean, I would squash them myself if there was a policy on the way merging is done mentioned.

Thanks!

@pfmoore
Copy link
Member

pfmoore commented Apr 19, 2024

We don't squash merge, that's our normal practice. I thought it was documented, but maybe it isn't - if so, I'm sorry. Don't worry about the messy commit history, perfect commits is not something we particularly strive for.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to install a module from svn if verbose mode is on
5 participants