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

Use fetch and checkout origin/main instead of pull #153

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions webapp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ projects:
languages: # list of language codes supported in the project
- sv
- de
base_branch: main # optional default to 'main'
```

Start the server with `npm run dev` in `webapp`.
Expand Down
24 changes: 12 additions & 12 deletions webapp/src/RepoGit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { getTranslationsBySourceFile } from '@/utils/translationObjectUtil';
import { Store } from '@/store/Store';

export class RepoGit {
private GIT_FETCH_TTL = 30_000; // 30 sec before fetch again
private static repositories: {
[name: string]: Promise<RepoGit>;
} = {};
Expand All @@ -36,7 +37,7 @@ export class RepoGit {
return RepoGit.repositories[key];
}
const repository = new RepoGit(spConfig);
const work = repository.checkoutBaseAndPull();
const work = repository.fetchAndCheckoutOriginBase();
const promise = work.then(() => repository);
RepoGit.repositories[key] = promise;

Expand All @@ -58,26 +59,25 @@ export class RepoGit {
debug(`Cloning repo: ${spConfig.repoPath} ...`);
await git.clone(spConfig.cloneUrl, spConfig.repoPath);
debug(`Cloned repo: ${spConfig.repoPath}`);
debug(`Checkout base branch: ${spConfig.baseBranch} ...`);
await git.checkout(spConfig.baseBranch);
debug(`Checked out base branch: ${spConfig.baseBranch}`);
debug(`Checkout base branch: ${spConfig.originBaseBranch} ...`);
await git.checkout(spConfig.originBaseBranch);
debug(`Checked out base branch: ${spConfig.originBaseBranch}`);
}

/**
* Checkout base branch and pull
* @returns base branch name
amerharb marked this conversation as resolved.
Show resolved Hide resolved
*/
public async checkoutBaseAndPull(): Promise<string> {
await this.git.checkout(this.spConfig.baseBranch);

public async fetchAndCheckoutOriginBase(): Promise<string> {
const now = new Date();
const age = now.getTime() - this.lastPullTime.getTime();
if (age > 30000) {
// We only pull if old
await this.git.pull();
if (age > this.GIT_FETCH_TTL) {
// We only fetch if old
await this.git.fetch();
await this.git.checkout(this.spConfig.originBaseBranch);
this.lastPullTime = now;
}
return this.spConfig.baseBranch;
return this.spConfig.originBaseBranch;
}

public async saveLanguageFiles(projectPath: string): Promise<string[]> {
Expand All @@ -101,7 +101,7 @@ export class RepoGit {
addFiles: string[],
commitMsg: string,
): Promise<void> {
await this.git.newBranch(branchName, this.spConfig.baseBranch);
await this.git.newBranch(branchName, this.spConfig.originBaseBranch);
await this.git.add(addFiles);
await this.git.commit(commitMsg);
await this.git.push(branchName);
Expand Down
4 changes: 2 additions & 2 deletions webapp/src/actions/createPullRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default async function sendPullRequest(
try {
syncLock.set(repoPath, true);
const repoGit = await RepoGit.getRepoGit(serverProjectConfig);
const baseBranch = await repoGit.checkoutBaseAndPull();
const baseBranch = await repoGit.fetchAndCheckoutOriginBase();
const langFilePaths = await repoGit.saveLanguageFiles(
serverProjectConfig.projectPath,
);
Expand Down Expand Up @@ -89,7 +89,7 @@ export default async function sendPullRequest(
serverProjectConfig.repo,
serverProjectConfig.githubToken,
);
await repoGit.checkoutBaseAndPull();
await repoGit.fetchAndCheckoutOriginBase();
return {
branchName,
pullRequestStatus: 'success',
Expand Down
4 changes: 2 additions & 2 deletions webapp/src/dataAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export async function accessLanguage(

await RepoGit.cloneIfNotExist(project);
const repoGit = await RepoGit.getRepoGit(project);
await repoGit.checkoutBaseAndPull();
await repoGit.fetchAndCheckoutOriginBase();
const lyraConfig = await repoGit.getLyraConfig();
const projectConfig = lyraConfig.getProjectConfigByPath(project.projectPath);
const projectStore = await Store.getProjectStore(projectConfig);
Expand All @@ -63,7 +63,7 @@ export async function accessLanguage(
async function readProject(project: ServerProjectConfig) {
await RepoGit.cloneIfNotExist(project);
const repoGit = await RepoGit.getRepoGit(project);
await repoGit.checkoutBaseAndPull();
await repoGit.fetchAndCheckoutOriginBase();
const lyraConfig = await repoGit.getLyraConfig();
const projectConfig = lyraConfig.getProjectConfigByPath(project.projectPath);
const store = await Store.getProjectStore(projectConfig);
Expand Down
2 changes: 1 addition & 1 deletion webapp/src/utils/git/IGit.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export interface IGit {
clone(repoPath: string, localPath: string): Promise<void>;

pull(): Promise<void>;
fetch(): Promise<void>;

checkout(branch: string): Promise<void>;

Expand Down
4 changes: 2 additions & 2 deletions webapp/src/utils/git/SimpleGitWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ export class SimpleGitWrapper implements IGit {
await this.git.clone(repoPath, localPath);
}

public async pull(): Promise<void> {
await this.git.pull();
public async fetch(): Promise<void> {
await this.git.fetch();
}

public async checkout(branch: string): Promise<void> {
Expand Down
2 changes: 2 additions & 0 deletions webapp/src/utils/serverConfig.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('ServerConfig', () => {
expect(config.projects[0].name).toEqual('foo');
expect(config.projects[0].repoPath).toEqual('/path/to/repo');
expect(config.projects[0].baseBranch).toEqual('fooBranch');
expect(config.projects[0].originBaseBranch).toEqual('origin/fooBranch');
expect(config.projects[0].projectPath).toEqual('project'); // Note: path changed after normalization
expect(config.projects[0].owner).toEqual('owner');
expect(config.projects[0].repo).toEqual('app.zetkin.org');
Expand Down Expand Up @@ -103,6 +104,7 @@ describe('ServerConfig', () => {
const projectConfig = await ServerConfig.getProjectConfig('bar');
expect(projectConfig.repoPath).toEqual('/path/to/repo');
expect(projectConfig.baseBranch).toEqual('main'); // Note: default value when missed in config
expect(projectConfig.originBaseBranch).toEqual('origin/main'); // Note: default value when missed in config
expect(projectConfig.projectPath).toEqual('project2');
});

Expand Down
7 changes: 5 additions & 2 deletions webapp/src/utils/serverConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,21 @@ export class ServerConfig {
}

export class ServerProjectConfig {
public readonly originBaseBranch: string;
constructor(
public readonly name: string,
/** absolute local path to repo */
public readonly repoPath: string,
/** following GitHub terminology target branch called base branch */
/** following GitHub terminology target branch (typically named 'main' or 'master') called base branch */
public readonly baseBranch: string,
/** relative path of project from repo_path */
public readonly projectPath: string,
public readonly owner: string,
public readonly repo: string,
public readonly githubToken: string,
) {}
) {
this.originBaseBranch = `origin/${this.baseBranch}`;
}

public get cloneUrl(): string {
// TODO: support other provider than github
Expand Down