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

Use Yarn on Travis #453

Merged
merged 3 commits into from
Jan 10, 2019
Merged

Use Yarn on Travis #453

merged 3 commits into from
Jan 10, 2019

Conversation

pomek
Copy link
Member

@pomek pomek commented Nov 7, 2018

Suggested merge commit message (convention)

Other: Changed scripts in the @ckeditor/ckeditor5-dev-tests package after switching development environment to Yarn. See ckeditor/ckeditor5#1214.

@coveralls
Copy link

coveralls commented Jan 7, 2019

Coverage Status

Coverage remained the same at 89.147% when pulling cf905b7 on ckeditor5/t/1214 into 9a7f768 on master.

@@ -2,15 +2,18 @@

set -e

npm install mgit2 lerna@^2.0.0-rc.1 eslint-config-ckeditor5 karma-coveralls@^1.1.2
npm install -g mgit2
Copy link
Member

Choose a reason for hiding this comment

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

Why global?

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIR Yarn will remove this dependency (during yarn install) because it isn't listed in package.json.

Copy link
Contributor

Choose a reason for hiding this comment

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

In CF we have the mgit with its version listed in the package.json. Not sure if it will work here though.

Copy link
Member

Choose a reason for hiding this comment

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

We were avoiding relying on deps in package.json because doing npm i and then lerna bootstrap would install the same deps twice (so it'd be slow). That's why we had the deps listed here. I think it's still a thing here because e.g. karma-coverals is not needed in all the packages in our local dev envs. It's only needed on CI. And it's the opposite with the other dependencies – you don't want to install them from npm.

Copy link
Member Author

Choose a reason for hiding this comment

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

karma-coverals isn't a dep of any of ckeditor5-*. I added it as a dep of ckeditor5-dev-tests because coveralls is a part of the testing environment.

Co-Authored-By: pomek <pomek@users.noreply.github.com>
@Reinmar Reinmar merged commit 18b0dd0 into master Jan 10, 2019
@Reinmar Reinmar deleted the ckeditor5/t/1214 branch January 10, 2019 09:22
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.

4 participants