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

[DiffTrain] Add github url for the commit to the commit message #25845

Merged
merged 1 commit into from
Dec 8, 2022

Conversation

poteto
Copy link
Member

@poteto poteto commented Dec 7, 2022

Currently we just append the ref for the commit, let's make it clickable for easier debugging in syncs.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Dec 7, 2022
Copy link
Contributor

@sophiebits sophiebits left a comment

Choose a reason for hiding this comment

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

Would “commits” to show the log be more useful?

@sizebot
Copy link

sizebot commented Dec 7, 2022

Comparing: bfcbf33...bede698

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 154.37 kB 154.37 kB = 48.97 kB 48.97 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 156.29 kB 156.29 kB = 49.63 kB 49.63 kB
facebook-www/ReactDOM-prod.classic.js = 533.12 kB 533.12 kB = 94.96 kB 94.96 kB
facebook-www/ReactDOM-prod.modern.js = 518.22 kB 518.22 kB = 92.76 kB 92.76 kB
facebook-www/ReactDOMForked-prod.classic.js = 533.12 kB 533.12 kB = 94.96 kB 94.96 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against bede698

@poteto
Copy link
Member Author

poteto commented Dec 7, 2022

@sophiebits yay thanks :) do you mean including a changelog of all the commits included (like we had in the old sync script)? Our new DiffTrain integration syncs each commit individually so the log wouldn't be useful. Let me know if I'm misunderstanding though!

@sophiebits
Copy link
Contributor

In the URL if you do “commits” instead of “commit” then it will show the preceding commits as well. I was thinking that might be helpful if you’re checking a commit and trying to think of if some other particular commit preceded it / was included in that bundle. I’m sure you have a better mental picture though!

@poteto
Copy link
Member Author

poteto commented Dec 7, 2022

Ooh nice - let's add both!

@poteto poteto merged commit 1c7055d into facebook:main Dec 8, 2022
@poteto poteto deleted the lt/difftrain-url branch December 8, 2022 19:22
github-actions bot pushed a commit that referenced this pull request Dec 8, 2022
Currently we just append the ref for the commit, let's make it clickable
for easier debugging in syncs.

DiffTrain build for [1c7055d](1c7055d)
[View git log for this commit](https://github.com/facebook/react/commits/1c7055ddbbdae81224f998e42832d882fde06686)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants