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

Switch back to a caret range for history #2702

Merged
merged 3 commits into from
Dec 17, 2015
Merged

Switch back to a caret range for history #2702

merged 3 commits into from
Dec 17, 2015

Conversation

timdorr
Copy link
Member

@timdorr timdorr commented Dec 10, 2015

This should be published as 1.0.3. Should fix both the warnings from history (removed in 1.16.0) and any errors about unmet peerDependencies.

Also, note this is on the 1.0.x branch.

@mjackson
Copy link
Member

Thanks, @timdorr!

@mjackson
Copy link
Member

Wonder why the Travis build didn't run here...

@timdorr
Copy link
Member Author

timdorr commented Dec 10, 2015

We only build master

@mjackson
Copy link
Member

Ah, crap. We should build PRs too. Otherwise we'll never know when it's safe to merge.

@mjackson
Copy link
Member

It's my fault

@timdorr
Copy link
Member Author

timdorr commented Dec 10, 2015 via email

@mjackson
Copy link
Member

oh, wait. so i didn't need to do this?

@mjackson
Copy link
Member

maybe i just need to add 1.0.x to the only clause in the travis config?

@timdorr
Copy link
Member Author

timdorr commented Dec 10, 2015 via email

@mjackson
Copy link
Member

ok, I'm going to try pushing a modification to .travis.yml to this branch only. Then at least we can see how the build goes before we cut 1.0.3. Sorry for the confusion.

@timdorr
Copy link
Member Author

timdorr commented Dec 14, 2015

BTW, the errors we're seeing on Travis I have seen before when going to at least history 1.14.0, so it looks like the behavior of those APIs changed just enough to break the tests. Fixed tests were put in on this PR, so I can replicate that for 1.0.3.

@taion
Copy link
Contributor

taion commented Dec 14, 2015

@timdorr Can you cherry-pick in the test fixes for this PR then?

@timdorr
Copy link
Member Author

timdorr commented Dec 14, 2015

Yep, I will.

@mjackson
Copy link
Member

I think at least some of the errors were triggered by a bad expect release I made late last week. It's fixed now. I'm going to re-run the build.

@mjackson
Copy link
Member

Ah, maybe not :/ @timdorr Do you think the fixes in #2659 will help?

@timdorr
Copy link
Member Author

timdorr commented Dec 14, 2015

Yeah, since the fact that push/pushState was called with certain args isn't a good test. Testing the effects of that call is a good test. It's better in #2659.

@mjackson
Copy link
Member

ok, great. If you can cherry-pick and get the build green let's ship 1.0.3. Thanks, btw :)

@timdorr
Copy link
Member Author

timdorr commented Dec 14, 2015

Uh, I hate to be the bearer of bad news, but we missed a warning in 1.16.0. It's still coming up with how Link is implemented 😭

@timdorr
Copy link
Member Author

timdorr commented Dec 14, 2015

Bonus points: The warning is backwards. It should be testing that query is not defined. Whoops!

@taion
Copy link
Contributor

taion commented Dec 14, 2015

Oops.

@timdorr
Copy link
Member Author

timdorr commented Dec 14, 2015

If we can push remix-run/history#190 as 1.16.1, all the warnings should go away. But this should be good as-is for now.

taion added a commit that referenced this pull request Dec 17, 2015
Switch back to a caret range for history
@taion taion merged commit 17d7272 into 1.0.x Dec 17, 2015
@taion
Copy link
Contributor

taion commented Dec 17, 2015

Oops. Finger slipped. This should have been ^1.16.1, eh? Sorry.

@taion taion deleted the 1.0.3-maybe branch December 17, 2015 14:50
@timdorr
Copy link
Member Author

timdorr commented Dec 17, 2015

Not needed. Once 1.16.1 exists, then this 1.0.3 can be published.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants