-
Notifications
You must be signed in to change notification settings - Fork 6.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
Scaffold multi-package workspace. #6850
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
# See https://turbo.build/repo/docs/reference/command-line-reference/run#--force | ||
run: echo "turbo_args=--force=true --cache-dir=.turbo/cache" >> "$GITHUB_OUTPUT" |
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 is no longer needed as of Turborepo 2, since it's the default.
I looked into crowdin renames - this was the best I could find so far. https://community.crowdin.com/t/change-source-file-name-without-loosing-progress/7594/3 I will ask in our Slack channel with Crowdin to confirm or supply something more detailed |
Please keep this file in the root of the branch, it is used by Crowdin <-> GitHub connector: It would be required to update paths to EN files to be able to sync sources and translations: Please pause Crowdin <-> GitHub sync before merging the PR, then after it's merged Crowdin team will help to adapt the existing Crowdin project and configuration file to the new structure. |
Thanks, @Andrulko! I've remapped those paths now, if you'd like to check them to confirm I've done so correctly. |
@anthonyshew Just to make sure that everything would work fine, let's insert
Everything else looks good! Before you merge this branch, please pause GitHub sync in Crowdin (otherwise, it will upload new /apps/site folders along with files). I will help to move the existing files in Crowdin project to the new path and then we can safely resume the sync. No new files will be uploaded in this case 👌 |
Updated with that. Thank you! |
This comment was marked as resolved.
This comment was marked as resolved.
Running Lighthouse audit... |
Looking through the CI errors on this run, these are all what I would expect to see from the changes I've made running against the Actions from The Root Directory on Vercel will need to be updated to |
Gotcha! Ill give another review in a few minutes and Im fine with bearing the responsibilities of the merge. |
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.
It looks good to me. @bmuenzenmeyer note that Crowdin might break? But that's an issue to verify as a follow-up. I'm merging this now and going to observe what happens on the main branch
PR-URL: #53762 Refs: nodejs/nodejs.org#6850 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
PR-URL: nodejs#53762 Refs: nodejs/nodejs.org#6850 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
PR-URL: #53762 Refs: nodejs/nodejs.org#6850 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
PR-URL: #53762 Refs: nodejs/nodejs.org#6850 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
PR-URL: #53762 Refs: nodejs/nodejs.org#6850 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
PR-URL: #53762 Refs: nodejs/nodejs.org#6850 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
Description
Create the structure for a multi-package workspace for the Node.js website.
Core maintainers have expressed a desire for re-using code from the website in other places. These changes allows for those publishable packages to be created, since they can now be broken out to their own separate packages.
Unfortunately, this involves a large-scale cutover from the repository's previous structure, invalidating PR's that come from current
main
. I've received maintainer approval for my disruptiveness. 🙏Fortunately, those outstanding PRs can be fairly easily re-aligned with this PR, since I've made near-zero changes as far as the source code is concerned. This PR updates tooling, rather than functional code. (Prettier updated ~10 files that I saw at one point, but I don't believe this came from the changes I've made. I think the source code was somehow out-of-sync with the Prettier configuration.) If you'd like your PR to meet back up with this one, you should be able to move them from their current location to
./apps/site/<your-path>
.Validation
crowdin.yml
. I'm not familiar with that tool so I don't know if it makes sense to move into theapps/site
directory or if it needs to stay in the root.main
to here and have satisfied my initial attempts to break myself.site
, as desired. Just let me know!Anthony's Checklist
turbo
tasks run when invoked?