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

Prepare Changelog for 0.69.0 #33730

Closed
wants to merge 21 commits into from
Closed

Conversation

fortmarek
Copy link
Contributor

Summary

Changelog

[Internal] - Changelog for 0.69.0

Test Plan

@fortmarek fortmarek added the 📝 Changelog This identifies PRs that touch the changelog label Apr 28, 2022
@fortmarek fortmarek requested a review from hramos as a code owner April 28, 2022 12:35
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Shopify Partner: Shopify Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Apr 28, 2022
@analysis-bot
Copy link

analysis-bot commented Apr 28, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,787,815 -36,203
android hermes armeabi-v7a 7,176,712 -34,348
android hermes x86 8,097,660 -36,824
android hermes x86_64 8,075,515 -39,012
android jsc arm64-v8a 9,654,711 -36,236
android jsc armeabi-v7a 8,412,504 -34,385
android jsc x86 9,605,314 -36,857
android jsc x86_64 10,200,192 -39,042

Base commit: ea29ae1
Branch: main

@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@analysis-bot
Copy link

analysis-bot commented Jun 13, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: ea29ae1
Branch: main

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@fortmarek fortmarek force-pushed the changelog-0.69.0 branch 3 times, most recently from 6814d20 to 405d096 Compare June 15, 2022 07:05
@fortmarek
Copy link
Contributor Author

fortmarek commented Jun 15, 2022

This changelog is now ready from my side.

I have split the commits into individual categories if you want to read through the changes I have made. The initial commit includes whatever the changelog-generator has produced.

Feel free to add additional reviewers that you think should have a look at this.

As a side note, I have noticed some commits like this having two logs:

[General] [Added] - A fail proof check to catch any crash involving webview:
if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || (message!=null && (message.contains("WebView provider") || message.contains("No WebView installed"))))
[General] [Removed] - Flawed check to catch WebViewProvider crash:
if (message != null && exception.getClass().getCanonicalName().contains("MissingWebViewPackageException"))

The changelog-generator only generates a log for the first message. Additionally, whatever is after a line break is just ignored. So, the log for this specific message was in section ### Added with A fail proof check to catch any crash involving webview:.

We either might want to do a better job at linting the changelog messages in the PR process if we don't to allow these cases or update the changelog generator to generate the messages properly.

Copy link
Collaborator

@Simek Simek left a comment

Choose a reason for hiding this comment

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

Good job on the changelog formatting so far! 👍

I have left a few suggestions, do not marked all lines which need to be updated, but hope you get the gist of it. 🙂

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
fortmarek and others added 8 commits June 15, 2022 11:15
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
@fortmarek
Copy link
Contributor Author

Thanks @Simek for the thorough review! I will give this another pass now that I have a better idea what to look for.

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Generally looks good to me 👍 I'm importing this now, but we can keep on iterating on it for a bit

CHANGELOG.md Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

fortmarek and others added 2 commits June 16, 2022 14:09
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
CHANGELOG.md Outdated

### Breaking

- Remove console.disableYellowBox support ([b633cc1305](https://github.com/facebook/react-native/commit/b633cc130533f0731b2577123282c4530e4f0abe) by [@GijsWeterings](https://github.com/GijsWeterings))
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this match what we wrote on the blogpost, so having 4 bullet points?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those logs are in the Removed section. As @Simek has pointed out when discussing privately, having both
Breaking and Removed sections rarely makes sense and is confusing. I can move those points here to match the blog post.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think consistency with the blogpost has an higher priority 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 👌

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Lorenzo Sciandra <lorenzo.sciandra@gmail.com>
@fortmarek fortmarek requested review from Simek and kelset June 17, 2022 12:13
@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@cortinico
Copy link
Contributor

@kelset @fortmarek I've merged this. If there are modifications to do, please let me know 👌

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @fortmarek in d27c8cf.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📝 Changelog This identifies PRs that touch the changelog CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Shopify Partner: Shopify Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants