Skip to content

Refactor zipfetcher and consistently use rev, not baseref#598

Merged
eseliger merged 8 commits into
mainfrom
es/stable-repo-zip-fetcher
Sep 3, 2021
Merged

Refactor zipfetcher and consistently use rev, not baseref#598
eseliger merged 8 commits into
mainfrom
es/stable-repo-zip-fetcher

Conversation

@eseliger

@eseliger eseliger commented Sep 2, 2021

Copy link
Copy Markdown
Member

Before, sometimes we used Rev, sometimes BaseRef, which could in the worst case lead to inconsistencies. Search results might not match the fetched zip archive, or detected workspaces might differ from what's on disk in the end.

@eseliger eseliger force-pushed the es/stable-repo-zip-fetcher branch from 8c1c7ba to d534942 Compare September 2, 2021 22:31
Comment thread internal/batches/graphql/repository.go Outdated
}

return r.DefaultBranch.Name
return ensurePrefix(r.DefaultBranch.Name)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was inconsistent.

@eseliger eseliger force-pushed the es/stable-repo-zip-fetcher branch from d534942 to 0d1821a Compare September 3, 2021 00:26

@mrnugget mrnugget left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Didn't look too closely, but idea looks good to me! I like repozip as a package.

Comment thread internal/batches/repozip/fetcher.go Outdated
}
}

func SlugForPathInRepo(repoName, commit, path string) string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Random thought: instead of a util package, could we have a repository package? batches/repository.SlugForPath is a good name.

@eseliger eseliger force-pushed the es/stable-repo-zip-fetcher branch from fe3d712 to 0104d58 Compare September 3, 2021 09:01
@eseliger eseliger marked this pull request as ready for review September 3, 2021 09:15
@eseliger eseliger merged commit 5b47424 into main Sep 3, 2021
@eseliger eseliger deleted the es/stable-repo-zip-fetcher branch September 3, 2021 09:18
LawnGnome added a commit that referenced this pull request Oct 6, 2021
Removing this in #598 broke the `src batch repos` command, which used
the `url` field when rendering the link to the repo in Sourcegraph.
LawnGnome added a commit that referenced this pull request Oct 6, 2021
Removing this in #598 broke the `src batch repos` command, which used
the `url` field when rendering the link to the repo in Sourcegraph.
scjohns pushed a commit that referenced this pull request Apr 24, 2023
Before, sometimes we used Rev, sometimes BaseRef, which could in the worst case lead to inconsistencies. Search results might not match the fetched zip archive, or detected workspaces might differ from what's on disk in the end.
scjohns pushed a commit that referenced this pull request Apr 24, 2023
Removing this in #598 broke the `src batch repos` command, which used
the `url` field when rendering the link to the repo in Sourcegraph.
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.

2 participants