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

Allow setting of branch in request #85

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

m42e
Copy link

@m42e m42e commented Dec 21, 2022

No description provided.

app.js Outdated Show resolved Hide resolved
@aslushnikov
Copy link
Owner

could you please also update the README to include new argument?

with integration in clone command
@@ -75,7 +75,7 @@ class DownloadManager {
* @param {string} url
* @return {!Downloader}
*/
createGitDownloader(url) {
createGitDownloader(url, branch) {
Copy link
Owner

Choose a reason for hiding this comment

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

could you please add the @param {string} branch to the jsdoc above?

@@ -86,7 +86,7 @@ class DownloadManager {
var fulfill;
var promise = new Promise(x => fulfill = x);
logger.info(`git clone ${url} ${folderPath}`);
exec('git', ['clone', '--depth', '1', url, folderPath], {timeout: utils.minutes(2)}, (err, stdout, stderr) => {
exec('git', ['clone', '--depth', '1', '--branch', branch, url, folderPath], {timeout: utils.minutes(2)}, (err, stdout, stderr) => {
Copy link
Owner

Choose a reason for hiding this comment

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

this would be a breaking behavior as well.

  • Before, we would always clone default branch, no matter its name.
  • Now, we would be checking out the master branch unless the branch is explicitly passed.

This means all current latexonline links to repositories with a non-master default branch would break.

Instead, can we use the --branch argument for cloning only when an explicit branch was supplied?

Also, IIRC we can save some time by also using the --single-branch argument together with --branch for the git command.

Copy link
Author

Choose a reason for hiding this comment

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

--depth 1 implies --single-branch, so this wouldn't be needed.

You are right, this will change default behaviour if the branch is not master.

BUT: The default behaviour was broken. The CompilationRequest always tried to check the master for the current commit.

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