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

Skip empty sync commits (both repos) #29707

Merged
merged 5 commits into from
Jun 3, 2024

Conversation

rickhanlonii
Copy link
Member

Requires #29706

The strategy here is to:

  • Checkout the builds/facebook-www branch
  • Read the current sync'd VERSION
  • Checkout out main and sync new build
  • sed/{new version string}/{old version string}
  • Run git status, skip sync if clean
  • Otherwise, sed/{old version string}/{new version string} and push commit

This means that:

  • We're using the real version strings from the builds
  • We are checking the last commit on the branch for the real last version
  • We're skipping any commits that won't result in changes
  • ???
  • Profit!

Copy link

vercel bot commented Jun 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 2, 2024 4:18pm

@react-sizebot
Copy link

react-sizebot commented Jun 1, 2024

Comparing: d77dd31...554476c

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.js = 6.66 kB 6.66 kB +0.05% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 496.48 kB 496.48 kB = 88.87 kB 88.87 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 501.30 kB 501.30 kB = 89.56 kB 89.56 kB
facebook-www/ReactDOM-prod.classic.js = 593.97 kB 593.97 kB = 104.48 kB 104.48 kB
facebook-www/ReactDOM-prod.modern.js = 570.35 kB 570.35 kB = 100.89 kB 100.89 kB
test_utils/ReactAllWarnings.js Deleted 63.65 kB 0.00 kB Deleted 15.90 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 63.65 kB 0.00 kB Deleted 15.90 kB 0.00 kB

Generated by 🚫 dangerJS against 554476c

.github/workflows/commit_artifacts.yml Show resolved Hide resolved
id: get_last_version
run: |
VERSION_CLASSIC=$(cat ./compiled/facebook-www/VERSION_CLASSIC)
VERSION_MODERN=$(cat ./compiled/facebook-www/VERSION_MODERN)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a safety check here to make sure the version number is well formed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, not sure about that, it would be nice to change the format without breaking the sync.

git status
echo "Status to check"
git status --porcelain | grep -qv '/REVISION$'
if git status --porcelain | grep -qv '/REVISION'; then
Copy link
Contributor

Choose a reason for hiding this comment

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

How come you have $ above but not here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The www file is REVISION or REVISION_TRANSFORMS but the fbsource one is just REVISION

@rickhanlonii rickhanlonii changed the title Skip empty sync commits in www Skip empty sync commits (both repos) Jun 2, 2024
@rickhanlonii rickhanlonii merged commit 5ad2c37 into facebook:main Jun 3, 2024
43 checks passed
@rickhanlonii
Copy link
Member Author

Ah crap, I meant to merge the other diff. @yungsters I think I addressed everything in your review, but if you want to review the new changes here I can submit a PR with more changes.

github-actions bot pushed a commit that referenced this pull request Jun 3, 2024
Requires #29706

The strategy here is to:
- Checkout the builds/facebook-www branch
- Read the current sync'd VERSION
- Checkout out main and sync new build
- sed/{new version string}/{old version string}
- Run git status, skip sync if clean
- Otherwise, sed/{old version string}/{new version string} and push
commit

This means that:
- We're using the real version strings from the builds
- We are checking the last commit on the branch for the real last
version
- We're skipping any commits that won't result in changes
- ???
- Profit!

DiffTrain build for commit 5ad2c37.
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