Skip to content

Conversation

@jackjocross
Copy link
Contributor

@jackjocross jackjocross commented Jan 11, 2017

Fixes #8744

@gaearon
Copy link
Collaborator

gaearon commented Jan 11, 2017

I don’t think those were linting errors, they were warnings.

Changes not specific to 15-stable should go against master.
Otherwise it’ll be confusing when we merge next time, and we’ll also lose them with 16.

"gulp-util": "^3.0.7",
"gzip-js": "~0.3.2",
"jest": "^15.1.1",
"jest-cli": "^15.1.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that just adding jest-cli is not enough.

The problem is we haven't synced package.json for a while, and we need to update it to match what we have on master. For example we already have jest-cli@18 there.

In general, in this PR the goal is to bring it as close as possible to master. We probably missed some commits at some point. There shouldn't be much intentional divergence between 15-stable and master. So now that you found a set of changes that work, could you also reduce it so that it matches master whenever possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In terms of linting, would it be preferable to pull in the eslint updates from master and make 15-stable conform? Or update 15-stable to conform to its current linting rules? I seem to be getting linting errors on install due to these changes and this rule. That rule was removed in 16.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It’s best to make it as close to master as we can in terms of linting setup. It would make sense to me to have 32f5b03 and similar changes cherry-picked.

@gaearon
Copy link
Collaborator

gaearon commented Jan 23, 2017

Superseded by #8778.

@gaearon gaearon closed this Jan 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants