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

libflux/flog: Support output to stderr #1192

Merged
merged 1 commit into from
Sep 15, 2017

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Sep 14, 2017

Make flux handle optional in logging functions. If flux handle
is NULL, output to stderr.

Add unit tests appropriately.

Fixes #1191

@chu11
Copy link
Member Author

chu11 commented Sep 14, 2017

I think primary review is on format of output:

return fprintf (stderr, "%s: %s\n", lstr, buf);

No colon? Upper case priority level string? I elected on colon and lower case.

@garlick
Copy link
Member

garlick commented Sep 14, 2017

Looks good! Don't forget a mention in flux_log(3).

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 7fd9a3b on chu11:issue1191 into ** on flux-framework:master**.

@chu11
Copy link
Member Author

chu11 commented Sep 15, 2017

Hmm, I had one build failure

PASS: t0001-basic.t 53 - instance can stop cleanly with subscribers (#1025)
ERROR: t0001-basic.t - missing test plan
ERROR: t0001-basic.t - exited with status 1

Which suggests my test seg-faulted or something like that. hmmm.

@garlick
Copy link
Member

garlick commented Sep 15, 2017

No colon? Upper case priority level string? I elected on colon and lower case.

Sounds ok to me.

Make flux handle optional in logging functions.  If flux handle
is NULL, output to stderr.

Update documentation and add unit tests appropriately.

Fixes flux-framework#1191
@codecov-io
Copy link

codecov-io commented Sep 15, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@06a2ec8). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master    #1192   +/-   ##
=========================================
  Coverage          ?   77.81%           
=========================================
  Files             ?      158           
  Lines             ?    29288           
  Branches          ?        0           
=========================================
  Hits              ?    22789           
  Misses            ?     6499           
  Partials          ?        0
Impacted Files Coverage Δ
src/common/libflux/flog.c 93.49% <100%> (ø)

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 670fbaa on chu11:issue1191 into ** on flux-framework:master**.

@chu11
Copy link
Member Author

chu11 commented Sep 15, 2017

pushed manpage updates & pushed fix for issue I listed above (failed chain lint was the issue).

first run through travis hit #1169 and some write errors, restarting

@garlick
Copy link
Member

garlick commented Sep 15, 2017

Thanks! Looks good.

@garlick garlick merged commit 3cd405f into flux-framework:master Sep 15, 2017
@grondo grondo mentioned this pull request May 10, 2018
@chu11 chu11 deleted the issue1191 branch June 5, 2021 17:59
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.

4 participants