feat: Introduce --fixup to change subcommand #1079
Conversation
When working on a change in a monorepo, I will often need to add subsequent change files as the change spreads into multiple packages. This creates multiple changefiles as well as multiple commits. To tackle this, this change introduces a `--fixup` flag, modeled after the `--fixup` flag in `git commit`. When used, this will enable fix-up mode, which will: 1. Find the most recent commit which adds a change file 2. Add the new changes into that file 3. Commit with a `--fixup <sha>` flag If no commits are found in the current branch, then the standard flow is used to create a new commit / changefile.
ecraig12345
left a comment
ecraig12345
left a comment
There was a problem hiding this comment.
This is a good idea but needs a few updates for behavior consistency between grouped/ungrouped and avoiding duplicated logic.
| */ | ||
| async function findForkPoint(cwd: string): Promise<string | null> { | ||
| // Try common main branch names | ||
| const mainBranches = ['origin/main', 'origin/master', 'main', 'master']; |
There was a problem hiding this comment.
This should use BeachballOptions.branch as the comparison basis
| ...newChange, | ||
| // Override with merged comment and higher change type | ||
| comment: existingChange.comment ? `${existingChange.comment}\n\n${newChange.comment}` : newChange.comment, | ||
| type: getHigherChangeType(existingChange.type, newChange.type), |
There was a problem hiding this comment.
This should use the existing getMaxChangeType helper
| configPath?: string; | ||
| dependentChangeType?: ChangeType; | ||
| /** | ||
| * For the change command: update the most recently added change file instead of creating a new one, |
There was a problem hiding this comment.
Can you add a note that this doesn't override the usual logic for whether change files are needed? (specifically for any package which already has a change file, unless --package is specified, it does nothing)
| // Search through commits from newest to oldest | ||
| for (const commitHash of commits) { | ||
| // Check what files were added in this commit (compare with parent) | ||
| const diffResult = await gitAsync( |
There was a problem hiding this comment.
There's already some closely related logic in getChangedPackages, so it would be best if at least some of that could be reused/combined with the logic here.
| } | ||
| } | ||
|
|
||
| if (addedChangeFiles.length > 0) { |
There was a problem hiding this comment.
It seems like this will only work properly with grouped change files? For ungrouped change files, you either need to find the right file for the specific package, or just create a new file and add it to the commit with change files. Creating a new file is most similar to what you're doing with grouped changes.
|
|
||
| // Get commits on the current branch, limited to MAX_COMMITS | ||
| const logResult = await gitAsync( | ||
| ['log', '--oneline', '--format=%H', `-n`, MAX_COMMITS.toString(), `${forkPointResult}..HEAD`], |
There was a problem hiding this comment.
It should be possible to log only commits touching changePath and filter by add/modify/rename -- e.g. git log --diff-filter=AMR origin/main..HEAD -- change (plus the args you already have). Then for grouped mode, you'd take the first change file from that commit, or for ungrouped mode you'd add a new file to that commit.
| fs.writeFileSync(changeFilePath, JSON.stringify(groupedChanges, null, 2)); | ||
| } else { | ||
| // For individual change files, merge the new change with the existing one | ||
| // We assume there's only one new change for fixup mode with individual files |
There was a problem hiding this comment.
I think in this case it's better to just add new change files rather than modifying an existing one--that's the equivalent of what you're doing in grouped mode. Or if the intent was to modify the existing change for a package (if present), you'd need some updates for both the grouped and ungrouped cases to handle that consistently.
Also I think the current logic is overwriting whichever package's change file was found first, which might not be the correct one for that package. (Most multi-package repos use grouped change files, but there are a few that don't.)
2 participants
When working on a change in a monorepo, I will often need to add subsequent change files as the change spreads into multiple packages. This creates multiple changefiles as well as multiple commits.
To tackle this, this change introduces a
--fixupflag, modeled after the--fixupflag ingit commit. When used, this will enable fix-up mode, which will:--fixup <sha>flagIf no commits are found in the current branch, then the standard flow is used to create a new commit / changefile.