Skip to content

Commit 5481b2b

Browse files
samcoesamcoe
committed
Move dirty repo check before any network calls
1 parent 8219710 commit 5481b2b

File tree

2 files changed

+16
-27
lines changed

2 files changed

+16
-27
lines changed

‎pkg/cmd/repo/sync/sync.go‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ func syncLocalRepo(opts *SyncOptions) error {
106106
return err
107107
}
108108

109+
dirtyRepo, err := opts.Git.IsDirty()
110+
if err != nil {
111+
return err
112+
}
113+
if dirtyRepo {
114+
return fmt.Errorf("can't sync because there are local changes, please commit or stash them")
115+
}
116+
109117
if opts.Branch == "" {
110118
httpClient, err := opts.HttpClient()
111119
if err != nil {
@@ -127,9 +135,6 @@ func syncLocalRepo(opts *SyncOptions) error {
127135
if errors.Is(err, divergingError) {
128136
return fmt.Errorf("can't sync because there are diverging changes, you can use `--force` to overwrite the changes")
129137
}
130-
if errors.Is(err, dirtyRepoError) {
131-
return fmt.Errorf("can't sync because there are local changes, please commit or stash them")
132-
}
133138
return err
134139
}
135140

@@ -209,8 +214,7 @@ func syncRemoteRepo(opts *SyncOptions) error {
209214
return nil
210215
}
211216

212-
var divergingError = errors.New("diverging commits")
213-
var dirtyRepoError = errors.New("dirty repo")
217+
var divergingError = errors.New("diverging changes")
214218

215219
func executeLocalRepoSync(srcRepo ghrepo.Interface, opts *SyncOptions) error {
216220
// Remotes precedence by name
@@ -243,14 +247,6 @@ func executeLocalRepoSync(srcRepo ghrepo.Interface, opts *SyncOptions) error {
243247
}
244248
}
245249

246-
dirtyRepo, err := git.IsDirty()
247-
if err != nil {
248-
return err
249-
}
250-
if dirtyRepo {
251-
return dirtyRepoError
252-
}
253-
254250
startBranch, err := git.CurrentBranch()
255251
if err != nil {
256252
return err

‎pkg/cmd/repo/sync/sync_test.go‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,10 @@ func Test_SyncRun(t *testing.T) {
122122
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
123123
},
124124
gitStubs: func(mgc *mockGitClient) {
125+
mgc.On("IsDirty").Return(false, nil).Once()
125126
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
126127
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
127128
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(true, nil).Once()
128-
mgc.On("IsDirty").Return(false, nil).Once()
129129
mgc.On("CurrentBranch").Return("trunk", nil).Once()
130130
mgc.On("Merge", []string{"--ff-only", "refs/remotes/origin/trunk"}).Return(nil).Once()
131131
},
@@ -141,10 +141,10 @@ func Test_SyncRun(t *testing.T) {
141141
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
142142
},
143143
gitStubs: func(mgc *mockGitClient) {
144+
mgc.On("IsDirty").Return(false, nil).Once()
144145
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
145146
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
146147
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(true, nil).Once()
147-
mgc.On("IsDirty").Return(false, nil).Once()
148148
mgc.On("CurrentBranch").Return("trunk", nil).Once()
149149
mgc.On("Merge", []string{"--ff-only", "refs/remotes/origin/trunk"}).Return(nil).Once()
150150
},
@@ -162,10 +162,10 @@ func Test_SyncRun(t *testing.T) {
162162
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
163163
},
164164
gitStubs: func(mgc *mockGitClient) {
165+
mgc.On("IsDirty").Return(false, nil).Once()
165166
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
166167
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
167168
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(true, nil).Once()
168-
mgc.On("IsDirty").Return(false, nil).Once()
169169
mgc.On("CurrentBranch").Return("trunk", nil).Once()
170170
mgc.On("Merge", []string{"--ff-only", "refs/remotes/origin/trunk"}).Return(nil).Once()
171171
},
@@ -178,10 +178,10 @@ func Test_SyncRun(t *testing.T) {
178178
Branch: "test",
179179
},
180180
gitStubs: func(mgc *mockGitClient) {
181+
mgc.On("IsDirty").Return(false, nil).Once()
181182
mgc.On("Fetch", []string{"origin", "+refs/heads/test"}).Return(nil).Once()
182183
mgc.On("HasLocalBranch", []string{"test"}).Return(true).Once()
183184
mgc.On("IsAncestor", []string{"test", "origin/test"}).Return(true, nil).Once()
184-
mgc.On("IsDirty").Return(false, nil).Once()
185185
mgc.On("CurrentBranch").Return("test", nil).Once()
186186
mgc.On("Merge", []string{"--ff-only", "refs/remotes/origin/test"}).Return(nil).Once()
187187
},
@@ -199,10 +199,10 @@ func Test_SyncRun(t *testing.T) {
199199
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
200200
},
201201
gitStubs: func(mgc *mockGitClient) {
202+
mgc.On("IsDirty").Return(false, nil).Once()
202203
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
203204
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
204205
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(false, nil).Once()
205-
mgc.On("IsDirty").Return(false, nil).Once()
206206
mgc.On("CurrentBranch").Return("trunk", nil).Once()
207207
mgc.On("Reset", []string{"--hard", "refs/remotes/origin/trunk"}).Return(nil).Once()
208208
},
@@ -218,6 +218,7 @@ func Test_SyncRun(t *testing.T) {
218218
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
219219
},
220220
gitStubs: func(mgc *mockGitClient) {
221+
mgc.On("IsDirty").Return(false, nil).Once()
221222
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
222223
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
223224
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(false, nil).Once()
@@ -229,15 +230,7 @@ func Test_SyncRun(t *testing.T) {
229230
name: "sync local repo with parent and local changes",
230231
tty: true,
231232
opts: &SyncOptions{},
232-
httpStubs: func(reg *httpmock.Registry) {
233-
reg.Register(
234-
httpmock.GraphQL(`query RepositoryInfo\b`),
235-
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
236-
},
237233
gitStubs: func(mgc *mockGitClient) {
238-
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
239-
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
240-
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(true, nil).Once()
241234
mgc.On("IsDirty").Return(true, nil).Once()
242235
},
243236
wantErr: true,
@@ -253,10 +246,10 @@ func Test_SyncRun(t *testing.T) {
253246
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
254247
},
255248
gitStubs: func(mgc *mockGitClient) {
249+
mgc.On("IsDirty").Return(false, nil).Once()
256250
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
257251
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
258252
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(true, nil).Once()
259-
mgc.On("IsDirty").Return(false, nil).Once()
260253
mgc.On("CurrentBranch").Return("test", nil).Once()
261254
mgc.On("Checkout", []string{"trunk"}).Return(nil).Once()
262255
mgc.On("Merge", []string{"--ff-only", "refs/remotes/origin/trunk"}).Return(nil).Once()

0 commit comments

Comments
 (0)