refactor(stack sync): Don't pass StackSyncFlags to actions.SyncStack #136
Conversation
Current Aviator status
This PR was merged using Aviator. |
|
If we do this, shouldn't we remove the individual fetches? |
twavv
force-pushed
the
travis/mer-2353-av-cli-stack-sync-should-fetch-first
branch
from
18fa0f1
to
4eb9951
Compare
| } | ||
| ) | ||
|
|
||
| func WithSkipNextCommit() SyncStackOpt { |
There was a problem hiding this comment.
I'm not 100% on this but I thought doing the functional option style was a better choice than putting it in the state (since the state gets persisted to disk, but --skip is fundamentally a one-time thing.
There was a problem hiding this comment.
Using this style seems to me too much for internal package, but if you want to use this sure.
internal/actions/reparent.go
Outdated
| @@ -149,7 +149,7 @@ func ReparentSkipContinue( | |||
| } | |||
|
|
|||
| if output.ExitCode != 0 && strings.Contains(string(output.Stderr), "no rebase in progress") { | |||
| // If there's no rebase, assume the user did `git rebase --continue/--skip` manually. | |||
| // If there's no rebase, assume the user did `git rebase --continue/--skipNextCommit` manually. | |||
There was a problem hiding this comment.
There's no such option like --skipNextCommit.
internal/actions/pr.go
Outdated
| @@ -633,7 +633,7 @@ func ParsePRMetadata( | |||
| return | |||
| } | |||
|
|
|||
| // This will skip over any data lines (since those weren't consumed by buf, | |||
| // This will skipNextCommit over any data lines (since those weren't consumed by buf, | |||
There was a problem hiding this comment.
I guess this is a mistake.
internal/actions/push.go
Outdated
| @@ -27,7 +27,7 @@ type PushOpts struct { | |||
| Force ForceOpt | |||
| // If true, require the corresponding branch exist on the remote (otherwise, don't push). | |||
| SkipIfRemoteBranchNotExist bool | |||
| // If true, skip pushing the branch if the corresponding branch on the remote points to the | |||
| // If true, skipNextCommit pushing the branch if the corresponding branch on the remote points to the | |||
internal/actions/sync_branch.go
Outdated
| @@ -24,7 +24,9 @@ type SyncBranchOpts struct { | |||
| // If specified, synchronize the branch against the latest version of the | |||
| // trunk branch. This value is ignored if the branch is not a stack root. | |||
| ToTrunk bool | |||
| Skip bool | |||
| // If true, skipNextCommit the current commit. | |||
There was a problem hiding this comment.
This is a mistake. s/skipNextCommit/skip/
| } | ||
| ) | ||
|
|
||
| func WithSkipNextCommit() SyncStackOpt { |
There was a problem hiding this comment.
Using this style seems to me too much for internal package, but if you want to use this sure.
twavv
changed the title
twavv
changed the title
aviator-app
bot
deleted the
travis/mer-2353-av-cli-stack-sync-should-fetch-first
branch
Just found this bug while doing some stuff locally.