-
Notifications
You must be signed in to change notification settings - Fork 9.3k
Test node6 on travis #551
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
Test node6 on travis #551
Conversation
utils/node6-transform/index.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't path.basename(curSource) === file?
utils/node6-transform/index.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
root is not used
utils/node6-transform/index.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do the path-joining magic in the very beginning so that it's easier to follow what's going on.
const from = path.join(source, file);
const to = path.join(target, file);
...
utils/node6-transform/index.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's check for file explicitly. It would not be nice to write over a symlink.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what you mean by this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean:
if (fs.lstatSync(curSource).isDirectory()) {
...
} else if (fs.lstatSync(curSource).isFile()) {
...
}
utils/node6-transform/index.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't copying everything into a node6/ work? this would spare you from this replace.
I.e.:
removeSync('node6');
copyFolder('lib', 'node6/lib');
copyFolder('test', 'node6/test');
copyFile('index.js', 'node6/index.js');There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gets really tricky because we load some utils from lib, and some utils are hard to convert. There are also paths for things like package.json and .local-chromium which get confusing.
No description provided.