kentcdodds / webpack-validator-DEPRECATED

Use this to save yourself some time when working on a webpack configuration.
MIT License
93 stars 8 forks source link

chore(travis): Run validate on travis and report coverage #30

Closed kentcdodds closed 8 years ago

kentcdodds commented 8 years ago

Closes #27

Thoughts @jonathanewerner and @sarbbottam?

My guess is that this will fail for node 0.10, but there are still a TON of people using that version and I'd love this to work for them too. I don't expect it to be too much work because we're transpiling anyway :-) But we may need to change a few things in the code to make it work.

I think we can merge this and work on those issues individually.

codecov-io commented 8 years ago

Current coverage is 100.00%

Branch #30 has no coverage reports uploaded yet.

No diff could be generated. No reports for master found. Review entire Coverage Diff as of 648e210

Powered by Codecov. Updated on successful CI builds.

kentcdodds commented 8 years ago

Looks like our first task after this gets merged is to update anywhere that we say: import foo from '.' to say: import foo from './index' because Node 0.10 doesn't support the dot reference.

Or maybe there's a babel plugin we could use (which would be sick). I wonder if there's a babel preset for Node 0.10. That would be a amazing...

jonathanglasmeyer commented 8 years ago

Yey cool! I don't have pain with replacing those dots with index. Should that refactoring take place on this branch or should I merge this?

sarbbottam commented 8 years ago

Looks like our first task after this gets merged is to update anywhere that we say: import foo from '.' to say: import foo from './index' because Node 0.10 doesn't support the dot reference.

Should we have a PR that replaces import foo from '.' to import foo from './index' merged first before this PR? I would ideally prefer to only merge a PR that would pass the build. :smile:

jonathanglasmeyer commented 8 years ago

I'm with you there. :)

kentcdodds commented 8 years ago

I would ideally prefer to only merge a PR that would pass the build.

¯\_(ツ)_/¯ I'm good with that :-)

kentcdodds commented 8 years ago

Looks like one of our validators is using a newer path API. We'll need a workaround.

sarbbottam commented 8 years ago

Could we use https://www.npmjs.com/package/path-parse?

import pathParse from 'path-parse'`

if (!path.parse) {
  path.parse = pathParse
}
kentcdodds commented 8 years ago

I would prefer monkey-patching and just use pathParse rather than path.parse personally.

jonathanglasmeyer commented 8 years ago

Opened #35 for this pending sub-task, would be a good first-timer I think. @kentcdodds not sure I'm correctly parsing your preference here: would like to prefer to use pathParse directly or conditionally overwrite ( = monkey-patch ? :)) path.parse with said function?

kentcdodds commented 8 years ago

I would prefer that we use it directly. I don't want to monkey patch it. :-)

kentcdodds commented 8 years ago

Rebased with the latest from master. Let's see if travis passes this time :-)

kentcdodds commented 8 years ago

ok, I think I've got it now :D

kentcdodds commented 8 years ago

QCbsHcryxPQGc