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

libsubprocess cleanup / renaming / refactoring #1713

Merged
merged 3 commits into from Oct 9, 2018

Conversation

Projects
None yet
4 participants
@chu11
Copy link
Contributor

chu11 commented Oct 9, 2018

@trws noted some confusion about the fact there is a libsubprocess and a subprocess library in flux-core. @SteVwonder suggested deprecating the old libsubprocess library, which seemed like a good idea. But since no code actually uses the old libsubprocess library anymore this seemed like a waste to code. So I removed the old code instead.

But we still need libsubprocess b/c of zio. With the subprocess code removed, I renamed libsubprocess to libzio, b/c that seemed to make sense.

Then for consistency, I renamed common/subprocess to common/libsubprocess, so it's officially converted into being the new libsubprocess.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 9, 2018

Nice cleanup!

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 9, 2018

Codecov Report

Merging #1713 into master will increase coverage by 0.05%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master    #1713      +/-   ##
==========================================
+ Coverage   79.26%   79.31%   +0.05%     
==========================================
  Files         185      184       -1     
  Lines       35042    34553     -489     
==========================================
- Hits        27775    27407     -368     
+ Misses       7267     7146     -121
Impacted Files Coverage Δ
src/cmd/builtin/proxy.c 74.75% <ø> (ø) ⬆️
src/broker/runlevel.c 80.89% <ø> (ø) ⬆️
src/common/libsubprocess/server.c 69.75% <ø> (ø)
src/common/libkz/kz.c 81.59% <ø> (ø) ⬆️
src/common/libsubprocess/local.c 76.27% <ø> (ø)
src/modules/cron/task.c 79.31% <ø> (ø) ⬆️
src/modules/wreck/wrexecd.c 76.22% <ø> (ø) ⬆️
src/modules/wreck/job.c 69.41% <ø> (ø) ⬆️
src/bindings/lua/flux-lua.c 82.12% <ø> (ø) ⬆️
src/common/libsubprocess/remote.c 69.37% <ø> (ø)
... and 9 more
@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Oct 9, 2018

Yeah, this seemed like the right approach. Thanks!

Looks good to me, ready to merge? Or do you want your other PR in first?

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Oct 9, 2018

Oh, oops as I posted that the other PR already went in.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 9, 2018

Sorry about that, looks like we were on this at the same time. @chu11, this just needs a rebase.

chu11 added some commits Oct 8, 2018

common/libsubprocess: Remove subprocess.[ch]
With new subprocess library, libsubprocess library is no longer
needed.  However, preserve zio library as it is still used.
common/libsubprocess: Rename libsubprocess
Given libsubprocess no longer deals with subprocesses, rename
libsubprocess library and directory to libzio.

@chu11 chu11 force-pushed the chu11:rmlibsubprocess branch from 6d545fd to 07730a0 Oct 9, 2018

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Oct 9, 2018

rebased

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Oct 9, 2018

hit a #1699 , restarted builder

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 9, 2018

Thanks!

@garlick garlick merged commit 9fd52be into flux-framework:master Oct 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.