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

Added postinstall script #250

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added postinstall script #250

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 4, 2016

Description

Added post install task

Issues

@jaumard
Copy link
Contributor

jaumard commented Nov 4, 2016

Hi :)
Yarn.lock shouldn't be under git in my opinion, it should be on git ignore file.
Also we don't want to run tests after install at all. And you command use npm where the issue talk about yarn ?? Don't see how this can fix the issue.

@ghost
Copy link
Author

ghost commented Nov 4, 2016

Issue Description #249
All trails tests need to be run from a yarn install as well as npm

So what tests should be run after a yarn/npm install?

On topic of the yarn.lock: https://yarnpkg.com/en/docs/yarn-lock#toc-check-into-source-control

All yarn.lock files should be checked into source control (e.g. git or mercurial). This allows Yarn to install the same exact dependency tree across all machines, whether it be your coworker’s laptop or a CI server.

@jaumard
Copy link
Contributor

jaumard commented Nov 4, 2016

Yes trails tests, this means tests on travisCI and appVeyor should install/launch tests once with npm and once with yarn to test both.

About the yarn.lock now I'm sure it doesn't have to be on git (thanks for the link). Here we don't want to have all same package version because it's an open framework not a project. By setting it on git you bypass package dependencies rules and we don't want this. It sure make sense on real project and it's a great feature!

@ghost
Copy link
Author

ghost commented Nov 4, 2016

Ok, then i misunderstood the requirements.

@jaumard
Copy link
Contributor

jaumard commented Nov 4, 2016

No problem, @tjwebb correct me if I'm wrong

@tjwebb
Copy link
Member

tjwebb commented Nov 7, 2016

Yarn.lock shouldn't be under git in my opinion, it should be on git ignore file.

This could go either way. I think the only issue here is that, if the yarn.lock file is checked in, then yarn and npm install will probably install different versions of stuff, unless we also maintain an npm shrinkwrap file that is identical to the yarn.lock file. I think overall this would actually be good, but we'd need to implement some sort of management system to verify this. I don't want to do this by hand...

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.

Trails should be tested with yarnpkg in addition to npm
3 participants