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

add XONSH_PIPEFAIL option to raise on piped subproc failure #5239

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Qyriad
Copy link
Contributor

@Qyriad Qyriad commented Nov 29, 2023

Closes #4351.

Let me know if there's a better way to implement or test this, and feel free to bikeshed the option name, of course.

For community

⬇️ Please click the 👍 reaction instead of leaving a +1 or 👍 comment

@Qyriad
Copy link
Contributor Author

Qyriad commented Nov 29, 2023

Ahh tests failed on Windows. I'll change this to a draft while I figure that out :)

@Qyriad Qyriad marked this pull request as draft November 29, 2023 00:57
@anki-code
Copy link
Member

hi @Qyriad! Thank you for this!
Could you please show the real life case for usage this here or somewhere in docs.
I believe it will help future users.
Thanks!

@Qyriad
Copy link
Contributor Author

Qyriad commented Dec 13, 2023

Ah, the test problem turned out to be really simple. Re: docs, that's a great idea yes, I'll work on that

@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (17a43bd) 67.17% compared to head (da7add7) 67.15%.
Report is 6 commits behind head on main.

Files Patch % Lines
xonsh/procs/pipelines.py 92.30% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5239      +/-   ##
==========================================
- Coverage   67.17%   67.15%   -0.03%     
==========================================
  Files         119      119              
  Lines       23116    23134      +18     
  Branches     4853     4859       +6     
==========================================
+ Hits        15529    15535       +6     
- Misses       6383     6394      +11     
- Partials     1204     1205       +1     
Flag Coverage Δ
macOS-latest 64.48% <92.85%> (+0.02%) ⬆️
ubuntu-latest 64.74% <92.85%> (+<0.01%) ⬆️
windows-latest 63.11% <92.85%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Qyriad
Copy link
Contributor Author

Qyriad commented Dec 13, 2023

Let me know how that documentation looks. If everything looks good I'll rebase this

@anki-code
Copy link
Member

Looks nice! Please add the mention also into https://github.com/xonsh/xonsh/blob/main/docs/bash_to_xsh.rst

@anki-code
Copy link
Member

anki-code commented May 20, 2024

Hey! Is it ready to review? Nice feature!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement pipefail and errexit
3 participants