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

warn (don't die) on uname errors #1

Merged
merged 1 commit into from
Sep 10, 2016

Conversation

rivy
Copy link
Contributor

@rivy rivy commented Sep 8, 2016

  • change fatal exception to warning and return of empty results for _uname()

.# Discussion

Local versions of uname may not support all options, which will cause errors upon
invocation. Previously, this killed the build. Now, the error output of uname will
be printed (by uname), followed by a warning entered into the build log. But the
build will continue. This allows builds on systems with oulder uname versions.

This was unnecessarily tripping up builds on my machine with an older version of uname. I didn't want to make a wholesale change, but it might be better to Capture::Tiny all the output and log the uname error output. If you're interested, I can refactor for that outcome.

- change fatal exception to warning and return of empty results for _uname()

.# Discussion

Local versions of `uname` may not support all options, which will cause errors upon
invocation. Previously, this killed the build. Now, the error output of `uname` will
be printed (by `uname`), followed by a warning entered into the build log. But the
build will continue. This allows builds on systems with oulder `uname` versions.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 80.208% when pulling 3bfef75 on rivy:fix/skip.uname.errors into 19eb7ef on kentnl:master.

@rivy
Copy link
Contributor Author

rivy commented Sep 8, 2016

I amended the commit to fix the test failure.

I missed seeing the actual Perl::Critic error within the flurry of '"Policy "Perl::Critic::Policy::..." is not installed.' messages.
😬

@kentfredric
Copy link
Member

Its probably worth noting that that's the only place _my_log_fatal is called in this module, and I must have added that function for a reason. The trick is trying to remember why :)

@kentfredric
Copy link
Member

Ok, yeah. Capturing the error output and streaming that to the log is probably helpful, I guess.

As to why the existing code does what it does, gonna chalk it up to "being young and stupid" 😉

@kentfredric kentfredric merged commit 3bfef75 into kentnl:master Sep 10, 2016
kentfredric added a commit that referenced this pull request Sep 10, 2016
kentfredric added a commit that referenced this pull request Sep 10, 2016
 [Bugfix]
 - Don't die when uname call fails.
 - To be honest, I'm still not sure why I was doing this in the first place.
 - Patch Patched (with thanks) by Roy Ivy II ( #1 )

 [Dependencies::Stats]
 - Dependencies changed since 1.004003, see misc/*.deps* for details
 - develop: (suggests: ↑1)
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.

None yet

3 participants