Skip to content
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

repo-updater: Small cleanup #17302

Merged
merged 1 commit into from
Jan 15, 2021
Merged

repo-updater: Small cleanup #17302

merged 1 commit into from
Jan 15, 2021

Conversation

ryanslade
Copy link
Contributor

Fix a typo and reduce nesting.

Fix a typo and reduce nesting.
@ryanslade ryanslade added this to the Cloud 2021-01-13 milestone Jan 15, 2021
@ryanslade ryanslade requested a review from a team January 15, 2021 08:38
@codecov
Copy link

codecov bot commented Jan 15, 2021

Codecov Report

Merging #17302 (12153f7) into main (08f0293) will decrease coverage by 0.00%.
The diff coverage is 53.33%.

@@            Coverage Diff             @@
##             main   #17302      +/-   ##
==========================================
- Coverage   52.14%   52.14%   -0.01%     
==========================================
  Files        1712     1712              
  Lines       85441    85442       +1     
  Branches     7596     7596              
==========================================
- Hits        44554    44552       -2     
- Misses      36972    36975       +3     
  Partials     3915     3915              
Flag Coverage Δ *Carryforward flag
go 51.21% <53.33%> (-0.01%) ⬇️
integration 30.59% <ø> (ø) Carriedforward from 08f0293
storybook 30.23% <ø> (ø) Carriedforward from 08f0293
typescript 54.39% <ø> (ø) Carriedforward from 08f0293
unit 34.87% <ø> (ø) Carriedforward from 08f0293

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
cmd/repo-updater/repoupdater/server.go 57.52% <53.33%> (-0.36%) ⬇️
...nal/campaigns/resolvers/changeset_apply_preview.go 58.97% <0.00%> (-0.86%) ⬇️

@@ -472,36 +472,39 @@ func (s *Server) repoLookup(ctx context.Context, args protocol.RepoLookupArgs) (
return s.remoteRepoSync(ctx, codehost, string(args.Repo))
}

// We don't sync private repos on demand
if repo.Private {
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the behaviour though, doesn't it? Before we were not syncing the repo, but we'd return a valid result in https://github.com/sourcegraph/sourcegraph/pull/17302/files#diff-736069860c34f463e5653fc6e82948c934a6771434326e0b79d0e11f63c20f72L513-L520

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right, good catch. I'll revert

ryanslade added a commit that referenced this pull request Jan 15, 2021
ryanslade added a commit that referenced this pull request Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants