Skip to content
This repository has been archived by the owner on May 11, 2018. It is now read-only.

Support target versions as strings #231

Merged
merged 3 commits into from
Apr 7, 2017
Merged

Support target versions as strings #231

merged 3 commits into from
Apr 7, 2017

Conversation

existentialism
Copy link
Member

@existentialism existentialism commented Mar 28, 2017

First part of #187.

Wanted to keep it relatively small, so only the following changes:

  • Added semverify and moved _extend to utils file ala [WIP] Add node's "engines" option #114
  • build-data now outputs string values for env versions
  • String target versions are now allowed via using semver to check if a plugin is required
  • Left backwards compatibility (though cases like plugin "node": 6.9 and target "node": 6.10 will behave differently) with numeric target versions, not sure if we should drop in 2.0?

We should follow this up with #114 then possibly add range support to targets ("chrome": "^52") if we want.

@yavorsky
Copy link
Member

Maybe semverify all targets' versions immediately? If there are no difference between '5' and 5, why we should log it as a different values? Also it would be good for the flow brunch to have the same version type during the whole preset process. 😏

@existentialism
Copy link
Member Author

existentialism commented Mar 30, 2017

"Few" more changes:

  • Normalizing (into semver) target versions now occur within getTargets, so they're strings all the way down.
  • Moved getTargets into targets-parser, and refactored it a bit.
  • isPluginRequired now assumes passed environments have already been parsed by getTargets, so it doesn't do checks or run getTargets itself
  • For cleaner debugging output, added some utils to drop extraneous .0 from versions

@existentialism existentialism force-pushed the issue187 branch 3 times, most recently from 3de93ac to 9f0f104 Compare March 30, 2017 18:43
@codecov-io
Copy link

codecov-io commented Mar 30, 2017

Codecov Report

❗ No coverage uploaded for pull request base (2.0@4063949). Click here to learn what that means.
The diff coverage is 90.9%.

Impacted file tree graph

@@          Coverage Diff           @@
##             2.0     #231   +/-   ##
======================================
  Coverage       ?   94.61%           
======================================
  Files          ?        6           
  Lines          ?      223           
  Branches       ?       64           
======================================
  Hits           ?      211           
  Misses         ?        9           
  Partials       ?        3
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø)
src/utils.js 83.87% <83.87%> (ø)
src/targets-parser.js 95% <95%> (ø)

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 4063949...c9529e6. Read the comment docs.

@@ -245,20 +245,22 @@ const generateData = (environments, features) => {
environments.forEach(env => {
const version = getLowestImplementedVersion(options, env);
if (version !== null) {
plugin[env] = version;
plugin[env] = version.toString();
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we semverify these?

Copy link
Member

@hzoo hzoo left a comment

Choose a reason for hiding this comment

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

we can drop _extends and just use Object.assign in 2.x (another pr is fine)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants