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

Misc shell/libsubprocess fixes/cleanups #2286

Merged
merged 3 commits into from Aug 6, 2019

Conversation

@chu11
Copy link
Contributor

commented Aug 6, 2019

A few cleanups that I peeled off of #2271's work.

chu11 added 3 commits Jul 31, 2019
When referencing an output stream such as stdout/stderr, use the
variable name "stream" intead of "name".
Do not set FLUX_SUBPROCESS_FLAGS_STDIO_FALLTHROUGH flag when io_cb
is not set, it is not a desired use case.
Instead of returning an error when flux_cmd_setopt() tries to set/change
an existing option, allow it to update the option.

Add unit tests accordingly.
@chu11 chu11 changed the title Misc cleanups Misc shell/libsubprocess fixes/cleanups Aug 6, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Aug 6, 2019

Codecov Report

Merging #2286 into master will decrease coverage by <.01%.
The diff coverage is 84.61%.

@@            Coverage Diff             @@
##           master    #2286      +/-   ##
==========================================
- Coverage   80.84%   80.84%   -0.01%     
==========================================
  Files         213      213              
  Lines       33484    33483       -1     
==========================================
- Hits        27069    27068       -1     
  Misses       6415     6415
Impacted Files Coverage Δ
src/shell/task.c 91.11% <100%> (+0.89%) ⬆️
src/cmd/flux-job.c 85.23% <100%> (ø) ⬆️
src/common/libsubprocess/command.c 70.46% <100%> (+0.08%) ⬆️
src/shell/io.c 78.4% <71.42%> (ø) ⬆️
src/modules/connector-local/local.c 73.26% <0%> (-0.29%) ⬇️
src/common/libflux/message.c 80.88% <0%> (+0.12%) ⬆️
@garlick

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

LGTM!

@garlick garlick merged commit f02ba6c into flux-framework:master Aug 6, 2019
4 checks passed
4 checks passed
Summary 1 potential rule
Details
codecov/patch 84.61% of diff hit (target 80.84%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +3.77% compared to 12facc9
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@garlick garlick referenced this pull request Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.