Skip to content
This repository has been archived by the owner on Oct 26, 2018. It is now read-only.

Make UPDATE_LOCATION follow FSA #208

Merged
merged 1 commit into from
Jan 17, 2016
Merged

Make UPDATE_LOCATION follow FSA #208

merged 1 commit into from
Jan 17, 2016

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Jan 17, 2016

Fix #206

Could use redux-actions to create the action creator, but that seems overkill

@timdorr
Copy link
Member

timdorr commented Jan 17, 2016

Thanks! This is a good start. Can you add the TRANSISTION actions as well?

@SimenB
Copy link
Contributor Author

SimenB commented Jan 17, 2016

Ah, didn't bother with it as I thought it was internal.

Use redux-actions, or still do it manually?

@timdorr
Copy link
Member

timdorr commented Jan 17, 2016

Spellchecker fail: TRANSITION actions.

@timdorr
Copy link
Member

timdorr commented Jan 17, 2016

Manually. I don't think we need to add another dependency.

@SimenB
Copy link
Contributor Author

SimenB commented Jan 17, 2016

@timdorr Done.

@timdorr
Copy link
Member

timdorr commented Jan 17, 2016

Awesome! Thank you!

timdorr added a commit that referenced this pull request Jan 17, 2016
Make UPDATE_LOCATION follow FSA
@timdorr timdorr merged commit 37e95e7 into reactjs:master Jan 17, 2016
@SimenB SimenB deleted the patch-1 branch January 17, 2016 21:02
@SimenB
Copy link
Contributor Author

SimenB commented Jan 17, 2016

@timdorr I didn't realize 2.0 was out of beta.

This is a breaking change, isn't it?
No references to the names of the properties in the docs or sample I could find, but people have still probably used location

@timdorr
Copy link
Member

timdorr commented Jan 17, 2016 via email

@SimenB
Copy link
Contributor Author

SimenB commented Jan 17, 2016

aight 😄

@jlongster
Copy link
Member

Well, not sure they are really internal, since people can listen to the UPDATE_LOCATION action. This is the sort of thing I hate about semver though, I don't want to release a whole 3.0.0 version just for this, especially since it's a quick change right after 2.0.0.

I don't really know how often people listen to that action, probably not much. We should definitely document it in the changelog though.

@SimenB
Copy link
Contributor Author

SimenB commented Jan 20, 2016

Want me to add a PR for the changelog?

Edit: it is in the changelog, never mind

@valscion
Copy link

Well, not sure they are really internal, since people can listen to the UPDATE_LOCATION action.

Yeah, we do that. Glad I checked on the changelog before upgrading from beta version as this would've broke our stuff 😅

I agree that semver sucks in these cases. But I also don't really care at all even if this library's version would change from 2.0 to 3.0 within 4 days. It only tips me off to read the changelog if the major version changes — if there's only one change, it shouldn't be a hassle to upgrade immediately 😄

I'm just glad I didn't get burned before reading this issue ¯_(ツ)_/¯

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.

4 participants