Skip to content

fix: coerce python version to semver#1

Closed
radiantly wants to merge 1 commit intoforwardemail:masterfrom
radiantly:semverfix
Closed

fix: coerce python version to semver#1
radiantly wants to merge 1 commit intoforwardemail:masterfrom
radiantly:semverfix

Conversation

@radiantly
Copy link
Copy Markdown

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #1 into master will increase coverage by 0.86%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #1      +/-   ##
==========================================
+ Coverage   80.95%   81.81%   +0.86%     
==========================================
  Files           1        1              
  Lines          21       22       +1     
==========================================
+ Hits           17       18       +1     
  Misses          4        4
Impacted Files Coverage Δ
index.js 81.81% <100%> (+0.86%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c6cf3b...6494dfa. Read the comment docs.

Copy link
Copy Markdown
Collaborator

@niftylettuce niftylettuce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@radiantly why did you add stdout here? python --version reports to stderr afaik for all versions.

@niftylettuce
Copy link
Copy Markdown
Collaborator

we'll also need to make the same PR/change in forwardemail/python-dkim-verify@c5db654

@radiantly
Copy link
Copy Markdown
Author

radiantly commented May 19, 2019

I copied the format from python-dkim-verify, where the silent flag was set to false when testing.

const silent = process.env.NODE_ENV !== 'test';
const options = { silent, async: true };

// ensure python v2.7+ or v3.5+
let version = exec('python --version', { silent });
version = (version.stdout ? version.stdout : version.stderr)
  .split(' ')[1]
  .trim();

What do you think?

@radiantly
Copy link
Copy Markdown
Author

@niftylettuce

@radiantly
Copy link
Copy Markdown
Author

ping @niftylettuce see my comment above

@niftylettuce
Copy link
Copy Markdown
Collaborator

Actually, I like semver.coerce better. I've fixed this in both packages we use internally and also version bumped them. Thanks so much @radiantly for the nice fix.

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.

Error running on Ubuntu 18.04 LTS

3 participants