-
-
Notifications
You must be signed in to change notification settings - Fork 76
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
chore: verify changes on allowedFiles according to the template #1048
chore: verify changes on allowedFiles according to the template #1048
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1048 +/- ##
==========================================
- Coverage 94.29% 94.27% -0.02%
==========================================
Files 97 97
Lines 5626 5626
Branches 455 455
==========================================
- Hits 5305 5304 -1
- Misses 320 321 +1
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
A good start, thanks for looking into this! I think the general strategy is mostly implemented well... but the strategy itself I think is going to need to be reworked. What do you think?
Btw, looks like there are some CI failures. You'll want to fix them up. |
21a6c06
to
b7878d9
Compare
Whoops, I pushed to the wrong branch working on another PR - sorry about that :) |
b7878d9
to
8b92ae4
Compare
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.
Progress! 🚀
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 looking great, nice work! I like the snapshot approach. And it's already found a bunch of discrepancies - filed inline to fix separately. Yay!
Requesting changes largely because the script never finishes running, and filtering out a few specific things from the snapshots. I left a few questions inline too.
…ad/create-typescript-app into fix/verify-changes-in-hydration
@JoshuaKGoldberg Let me know if this is ready for a 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.
Wonderful, thanks so much for all your hard work on this PR @Aslemammad! I think it looks great and will be very helpful keeping the last few files aligned to the templates as time goes on.
I'll apply a couple touchups myself for linking to upstream bugs & adding back bin/index.js
. But no more blockers. Yay!
- "packageManager": "pnpm@8.12.1", | ||
+ "packageManager": "pnpm@8.7.0", |
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.
Heh, and in CI:
2023-12-30T06:48:46.9914884Z },
2023-12-30T06:48:46.9915527Z - - "packageManager": "pnpm@8.12.1",
2023-12-30T06:48:46.9916260Z + - "packageManager": "pnpm@8.13.1",
2023-12-30T06:48:46.9916878Z + "packageManager": "pnpm@8.7.0",
This'll probably start failing as soon as a new pnpm version is out. Lovely. That can be a followup.
Thank you so much, happy to see it finally being merged! |
...very amusing. |
Oh, and: renaming the PR title to |
@all-contributors please add @Aslemammad for code.
|
I've put up a pull request to add @Aslemammad! 🎉 I couldn't determine any contributions to add, did you specify any contributions? |
Adds @Aslemammad as a contributor for code. This was requested by JoshuaKGoldberg [in this comment](#1048 (comment)) --------- Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com> Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
🎉 This is included in version v1.50.2 🎉 The release is available on: Cheers! 📦🚀 |
script/migrate-test-e2e.js
PR Checklist
status: accepting prs
Overview
We track the changes using createRootFiles and we watch the changes files based on the function result.