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

Report size of app bundles on PRs #28019

Closed
wants to merge 3 commits into from

Conversation

tido64
Copy link
Collaborator

@tido64 tido64 commented Feb 11, 2020

Summary

Report size of app bundles on PRs. See React Native Benchmark Suite for further discussion.

Changelog

[Internal] [Added] - Report size of app bundles on PRs

Test Plan

PRs should start seeing comments from a bot with app bundle sizes, given that they got built successfully.

@tido64 tido64 requested a review from hramos as a code owner February 11, 2020 12:45
@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: Microsoft Partner: Microsoft Partner labels Feb 11, 2020
@react-native-bot react-native-bot added the Type: Enhancement A new feature or enhancement of an existing feature. label Feb 11, 2020
@analysis-bot
Copy link

RNTester.app (iOS): 9846784 bytes

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@hramos
Copy link
Contributor

hramos commented Feb 12, 2020

Making a slight adjustment - moving octokit/rest from a devDependency in the root package.json, to the bots/package.json file should work as well. It also keeps us from having to add octokit to our internal yarn offline mirror (which is based off the root package.json).

I couldn't update bots/package.json in your PR so the latest CI run will likely fail, but you may safely ignore that. I already made these changes internally.

hramos pushed a commit to hramos/react-native that referenced this pull request Feb 12, 2020
Summary:
Report size of app bundles on PRs. See [React Native Benchmark Suite](react-native-community/discussions-and-proposals#186) for further discussion.

## Changelog

[Internal] [Added] - Report size of app bundles on PRs
Pull Request resolved: facebook#28019

Test Plan: PRs should start seeing comments from a bot with app bundle sizes, given that they got built successfully.

Differential Revision: D19859187

Pulled By: hramos

fbshipit-source-id: 48ba25903356b219135716f989a4a3c05140abfb
@tido64
Copy link
Collaborator Author

tido64 commented Feb 12, 2020

@hramos Thanks for reviewing this!

Don't we also have to add an extra step to install the dependencies under bots/? That's where I had it originally but it failed and I wasn't sure what the right approach is. Never mind! I see you've done just that.

Since you've bumped the version number, should we also fix the deprecation warnings?

hramos pushed a commit to hramos/react-native that referenced this pull request Feb 13, 2020
Summary:
Pull Request resolved: facebook#28041

Report size of app bundles on PRs. See [React Native Benchmark Suite](react-native-community/discussions-and-proposals#186) for further discussion.

## Changelog

[Internal] [Added] - Report size of app bundles on PRs
Pull Request resolved: facebook#28019

Test Plan: PRs should start seeing comments from a bot with app bundle sizes, given that they got built successfully.

Reviewed By: cpojer

Differential Revision: D19859187

Pulled By: hramos

fbshipit-source-id: 0eb510287ba2001895d66d038fc1e7b8a722d9a8
@hramos
Copy link
Contributor

hramos commented Feb 13, 2020

Merged! Yes, we should fix the deprecated warnings. I also filed #28043 as a related item that came up while testing this.

@tido64 tido64 deleted the tido/report-bundle-size branch February 13, 2020 06:39
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Feb 13, 2020
@facebook-github-bot
Copy link
Contributor

@hramos merged this pull request in 1b56292.

facebook-github-bot pushed a commit that referenced this pull request Feb 20, 2020
Summary:
Addresses deprecation warnings from `octokit/rest`. This is a follow up to #28019.

## Changelog

[Internal] [Fixed] - Address deprecation warnings from `octokit/rest`
Pull Request resolved: #28050

Test Plan: PRs should still get app bundle sizes report, but the warnings in the build logs should be gone.

Reviewed By: cpojer

Differential Revision: D20008805

Pulled By: hramos

fbshipit-source-id: 891d14fd9d55f217194a095d2736494416dacda2
@analysis-bot
Copy link

test

4 similar comments
@analysis-bot
Copy link

test

@analysis-bot
Copy link

test

@analysis-bot
Copy link

test

@analysis-bot
Copy link

test

osdnk pushed a commit to osdnk/react-native that referenced this pull request Mar 9, 2020
Summary:
Pull Request resolved: facebook#28041

Report size of app bundles on PRs. See [React Native Benchmark Suite](react-native-community/discussions-and-proposals#186) for further discussion.

## Changelog

[Internal] [Added] - Report size of app bundles on PRs
Pull Request resolved: facebook#28019

Test Plan: PRs should start seeing comments from a bot with app bundle sizes, given that they got built successfully.

Reviewed By: cpojer

Differential Revision: D19859187

Pulled By: hramos

fbshipit-source-id: 3920dc60e6fd073928388e6ae52fc2ba1bc745ac
osdnk pushed a commit to osdnk/react-native that referenced this pull request Mar 9, 2020
Summary:
Addresses deprecation warnings from `octokit/rest`. This is a follow up to facebook#28019.

## Changelog

[Internal] [Fixed] - Address deprecation warnings from `octokit/rest`
Pull Request resolved: facebook#28050

Test Plan: PRs should still get app bundle sizes report, but the warnings in the build logs should be gone.

Reviewed By: cpojer

Differential Revision: D20008805

Pulled By: hramos

fbshipit-source-id: 891d14fd9d55f217194a095d2736494416dacda2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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: Microsoft Partner: Microsoft Partner Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants