FEAT: added reset branch to ref command #1716
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
jamesqquick
force-pushed
the
james/update-clone-to-reset-main
branch
from
f2a3e93 to
86efc9c
Compare
| const checkoutSuccess = checkoutRef(repoDir, targetBranch); | ||
|
|
||
| // Reset current branch to the specified ref | ||
| if (checkoutSuccess) { | ||
| execSync(`git reset --hard ${ref}`, { | ||
| cwd: repoDir, | ||
| stdio: 'inherit', | ||
| encoding: 'utf8', | ||
| }); | ||
| console.log(`Reset ${targetBranch} to ${ref} successfully.`); | ||
| } |
There was a problem hiding this comment.
Can we rely on errors being thrown instead of boolean conditions? If checkoutSuccess is false, we essentially do "nothing".
There was a problem hiding this comment.
Do you mean checking the boolean condition of checkoutSuccess? Right now, checkoutRef function handles errors gracefully, meaning it doesn't surface it's errors to a caller. Because of that, I don't know how I would know if it ran successfully or not?
Right now, if checkoutSuccess is false the error handling and logging has already been handled in checkoutRef.
Does that address your question?
There was a problem hiding this comment.
Ummm... so to me CLI application logic flow should be handled by errors being thrown instead of conditional statements like you have currently. It allows us to surface errors and handle them through the catch statement, rather than if x, do y, else noop.
I think it should be something like this:
try {
checkoutRef(...);
execSync(...);
} catch (error) {
if (error instanceof CheckoutRefError) {
// handle fallback logic
}
// ...
}There was a problem hiding this comment.
Yeah I understand that. The way things are written already, this would mean reworking checkoutRef to throw the errors that it's already handling? Then, we have to handle that error every time it's called, which is only once for now and potentially replicating logging for it?
Co-authored-by: Chancellor Clark <[email protected]>
…to callers for checkoutRef and resetBranchtoRef
There was a problem hiding this comment.
Looks good, I met with James and functionally this PR does what we want; there are improvements we can make to automatically set the upstream of local main to pull from the branch deployed by OCC. But to get this across the finish line, I'm going to approve this one.
| export function resetBranchToRef(projectDir: string, ghRef: string): void { | ||
| execSync(`git reset --hard ${ghRef}`, { | ||
| cwd: projectDir, | ||
| stdio: 'inherit', | ||
| encoding: 'utf8', | ||
| }); | ||
| } |
There was a problem hiding this comment.
As discussed internally, in the unlikely event that a bad ghRef is passed, this will throw and kill the CLI process. This will leave the user running the CLI in a state where the repository was cloned successfully, but dependencies wouldn't be installed nor would the environment file be written (see code reference below)
catalyst/packages/create-catalyst/src/commands/create.ts
Lines 257 to 266 in ad8c86d
For now, we'll accept this since it's unlikely the ghRef provided by OCC will be rejected. That said, we do need to handle errors in a follow up PR.
⚡️🏠 Lighthouse reportLighthouse ran against https://catalyst-latest-gqaiu2jx2-bigcommerce-platform.vercel.app 🖥️ DesktopWe ran Lighthouse against the changes on a desktop and produced this report. Here's the summary:
📱 MobileWe ran Lighthouse against the changes on a mobile and produced this report. Here's the summary:
|
What/Why?
When moving from OCC to development, users should have the ability to check out a ref that is then set to the
mainbranch so they can continue smoothly in their development journey. This will put them in a state where they have the exact code they originally deployed in OCC but will be set to themainbranch.reset-mainboolean option for CLIghRefandresetMainare presentTesting
--reset-mainflag with the end result of being in themainbranch with code matching thegh-refprovided