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

Update Dependencies #409

Merged
merged 9 commits into from
Nov 7, 2018
Merged

Update Dependencies #409

merged 9 commits into from
Nov 7, 2018

Conversation

wagenet
Copy link
Member

@wagenet wagenet commented Sep 7, 2018

No description provided.

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Sorry for the delays here, left some inline comments

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@wagenet
Copy link
Member Author

wagenet commented Oct 1, 2018

Turns out that almost all the dependencies I upgraded depend on Node 6, so not sure if it makes sense to fix this PR. @rwjblue

@Turbo87
Copy link
Member

Turbo87 commented Nov 1, 2018

@wagenet #427 dropped Node 4 support, so this PR could be revived

@rwjblue
Copy link
Member

rwjblue commented Nov 2, 2018

@wagenet - Looks like we only have a yarn.lock conflict now, mind rebasing and fixing that up?

.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@wagenet
Copy link
Member Author

wagenet commented Nov 2, 2018

See #457 for some context on the failing tests.

@wagenet wagenet force-pushed the update-dependencies branch 2 times, most recently from c2f8dda to f387355 Compare November 2, 2018 20:39
@wagenet
Copy link
Member Author

wagenet commented Nov 2, 2018

@rwjblue so we're still blocked here because of the issue with isSettled :/

@rwjblue
Copy link
Member

rwjblue commented Nov 2, 2018

Lets skip that one test for now, then follow up on the isSettled thing in #457.

@wagenet wagenet force-pushed the update-dependencies branch 2 times, most recently from 77f557d to fceccdf Compare November 2, 2018 21:38
@wagenet
Copy link
Member Author

wagenet commented Nov 2, 2018

Hmm... seeing a few of the scenarios failing. Will have to investigate further next week.

@wagenet
Copy link
Member Author

wagenet commented Nov 5, 2018

@rwjblue finally passing!

return {
useYarn: true,
scenarios: [
{
name: 'ember-2.0',
env,
Copy link
Member

Choose a reason for hiding this comment

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

We cannot use the shared env here (Ember 2.0 doesn't support use without jQuery), maybe make a envWithJQuery and an envWithoutJQuery?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm... well we can use it, it just doesn't do anything meaningful, so I agree that it is confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Which scenarios are actually expected to have jQuery?

Copy link
Member

Choose a reason for hiding this comment

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

2.16 and higher should be tested without jquery, we should also add entries in Travis.yml and try config for 2.18 and 3.4

@rwjblue
Copy link
Member

rwjblue commented Nov 7, 2018

Some conflicts now also (sorry for the delays / run around here)...

@wagenet wagenet merged commit 458b822 into emberjs:master Nov 7, 2018
@wagenet wagenet deleted the update-dependencies branch November 7, 2018 18:36
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.

3 participants