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

Make the output from Dulwich command class behave like Git #6

Merged
merged 14 commits into from
Oct 16, 2023

Conversation

hsorby
Copy link
Member

@hsorby hsorby commented Oct 16, 2023

No description provided.

@hsorby hsorby requested a review from metatoaster October 16, 2023 02:47
@metatoaster
Copy link
Member

Given that we are unlikely to bring back support for Mercurial ever, we can probably drop the Mercurial related classes and tests to make this pass CI.

@hsorby
Copy link
Member Author

hsorby commented Oct 16, 2023

Sounds good to me.

@hsorby
Copy link
Member Author

hsorby commented Oct 16, 2023

Since we don't really worry about Mercurial I'll try a quick fix first.

@coveralls
Copy link

coveralls commented Oct 16, 2023

Coverage Status

coverage: 99.597% (+0.01%) from 99.586% when pulling 0c9597b on hsorby:main into 2dc6e0d on PMR2:master.

return_code = 0
if output[1]:
return_code = 1
return '\n'.join(output[0]).encode(), output[1], return_code
Copy link
Member

Choose a reason for hiding this comment

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

The Popen return code is captured in the third element (idx 2) of the tuple as per https://github.com/PMR2/pmr2.wfctrl/blob/master/src/pmr2/wfctrl/core.py#L304-L305 - just use that instead? Avoids the condition check and thus the coverage issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@metatoaster
Copy link
Member

Actually went to the core of the problem after I realized these workarounds don't make sense - the problem being the test set up helpers weren't updated to return the 3-tuple. This was fixed so the nonsense handling no need to happen. I pushed to my own fork and will be merging this in shortly after CI is done for that.

@metatoaster metatoaster merged commit 0c9597b into PMR2:master Oct 16, 2023
16 checks passed
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