-
Notifications
You must be signed in to change notification settings - Fork 47.3k
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 Changelog for v15.5.1-15.5.4 #9537
Conversation
This could really use extra code review attention since the history of these changes was a bit convoluted to follow. After talking to @bvaughn and @acdlite, we thought it might make sense to put the 'add-ons' changes in a separate change log. The other option, of including them in the main React change log, seemed the more confusing of the two. Also this commit is related to and somewhat blocked by facebook/prop-types#40 **what is the change?:** Adding the change log for recent patch versions of React. **why make this change?:** We missed this step in the flurry of releasing patches, and it's useful for folks who want info about what version to use. **test plan:** Visual inspection of the change log. **issue:** facebook#9443
ADD-ONS-CHANGELOG.md
Outdated
|
||
## Pure Render Mixin | ||
|
||
### 15.5.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this one 15.5.2 and Test Utils has 15.5.1? (Can't tell if either is significant, honestly.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to figure out either what version a change was released in, or at least what the latest version of each package is. The latest version of each of these is 15.5.2 and 15.5.1 respectively.
ADD-ONS-CHANGELOG.md
Outdated
### 15.5.3 | ||
|
||
* Fix react-addons-create-fragment package to export correct thing. [See #9383](https://github.com/facebook/react/pull/9383). | ||
* Envify and collapse create-fragment UMD. [See #9383](https://github.com/facebook/react/pull/9383). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This points to the same PR as above. Mistake?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good of you to notice, and yes it's intentional. A bunch of these changes were made in PR 9383.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right - thanks again for flagging!
ADD-ONS-CHANGELOG.md
Outdated
|
||
* Fix devDeps. [See #9383](https://github.com/facebook/react/pull/9383). | ||
* Add missing object-assign dependency. [See #9383](https://github.com/facebook/react/pull/9383). | ||
* Envify and collapse create-react-class UMD. [See #9383](https://github.com/facebook/react/pull/9383). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually these all point to the same PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think maybe you meant to link multiple bullets to PR #9385?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do that - also going to update this to use the '[@author] in [#PR/commit]' format.
ADD-ONS-CHANGELOG.md
Outdated
@@ -0,0 +1,59 @@ | |||
# React Add-Ons Change Log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to have this file long term? Since these are final versions ever (hopefully), it seems a bit redundant to add a new file that we’ll never change, for parts that were technically discontinued. Maybe we could put this up as a gist instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good - will move it to a gist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe let’s just keep it in the main changelog? It seems like they could fit in fine if grouped under React Addons for each release (see like we did before).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on later comments, will move this inline to the main Changelog.
ADD-ONS-CHANGELOG.md
Outdated
## Create Fragment | ||
|
||
### 15.5.3 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general we avoid deeply technical descriptions of changes.
I would prefer that instead of "envify and collapse" we would say something like
- Fixed the accidental size regression in UMD bundles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aaah nice - that is a simpler way of saying it. Will fix.
CHANGELOG.md
Outdated
@@ -1,3 +1,21 @@ | |||
## 15.5.4 (April 11, 2017) | |||
|
|||
* Fix compatibility with Enzyme by exposing `batchedUpdates` on shallow renderer. [See #9382](https://github.com/facebook/react/commit/69933e25c37cf5453a9ef132177241203ee8d2fd). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, we try to separate React, React DOM, and React Test Renderer changes.
Can we make it clearer which package each change corresponds to?
I also think it might be simpler to roll addon updates right into this list, and summarize them as React Addons bullet point list for each version instead of giving each addon a section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure - maybe that will be less confusing after all. Although then it's a bit tricky to map the add-on version to the React version. I guess I would annotate them in the section where React was using that version of the add-on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also will update all these to use the '[@author] in [#PR/commit]' format.
**what is the change?:** - Use the '[@author] in [#PR/commit]' format for annotations - Make annotations less technical, more clear - Move 'React Addons' updates into main changelog - Remove separate 'React Addons' changelog **why make this change?:** These changes each make things more clear and accurate. **test plan:** Visual inspection **issue:**
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good (with nits)
Thanks!
CHANGELOG.md
Outdated
|
||
### React Test Renderer | ||
|
||
* Fix compatibility with Enzyme by exposing `batchedUpdates` on shallow renderer. [See #9382](https://github.com/facebook/react/commit/69933e25c37cf5453a9ef132177241203ee8d2fd). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming we'll fix up the link format here too to match others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - missed that one. Will fix.
CHANGELOG.md
Outdated
|
||
### React Addons | ||
|
||
* Fix react-addons-create-fragment package to export correct thing. ([@gaearon](https://github.com/gaearon) in [#9385](https://github.com/facebook/react/pull/9383)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: lets use backticks for package names. We don't use them for proper names though (e.g. we either write React DOM or react-dom
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - good to know. Fixing now. :)
**what is the change?:** - Put backticks around package names - Reformat link to to commit in '([@user](...) in [#NNNN](...))' format - Remove newlines after subheaders; in the past we sometimes have included a newline after the subheader, but most recently it looks like we do not. - Add some missing punctuation. **why make this change?:** Consistency and aesthetics **test plan:** Visual inspection **issue:** facebook#9443
* Fix compatibility with Enzyme by exposing `batchedUpdates` on shallow renderer. ([@gaearon](https://github.com/gaearon) in [#9382](https://github.com/facebook/react/commit/69933e25c37cf5453a9ef132177241203ee8d2fd). | ||
|
||
|
||
## 15.5.3 (April 7, 2017) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK versions before 15.5.4 are using prop-types < 15.5.7 in the UMD, thus having the same critical issue. So we should make it clear all of them are effectively deprecated, and people should skip 15.5.0 to 15.5.3, and use at least 15.5.4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great - for now I'll add the same disclaimer from the 'prop-types' repo. Was also thinking of adding the 'yanked' annotation but since that would be a new convention will propose it in a follow-up PR.
**what is the change?:** Adding deprecation notice to some recent React versions. **why make this change?:** These versions of React use a version of `prop-types` that had a critical bug. We updated the dependency in React 15.5.4, and hopefully people will see this notice and update. **test plan:** Visual inspection **issue:** facebook#9537
This could really use extra code review attention since the history of
these changes was a bit convoluted to follow.
After talking to @bvaughn and @acdlite, we thought it might make sense
to put the 'add-ons' changes in a separate change log. The other option,
of including them in the main React change log, seemed the more
confusing of the two.
Also this commit is related to and somewhat blocked by
facebook/prop-types#40
Also - I will update the release tags after we land some version of a changelog.
what is the change?:
Adding the change log for recent patch versions of React.
why make this change?:
We missed this step in the flurry of releasing patches, and it's useful
for folks who want info about what version to use.
test plan:
Visual inspection of the change log.
issue:
#9443