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

fix: set depth to 1 when fetching #268

Closed
wants to merge 1 commit into from

Conversation

winterqt
Copy link

This reduces the backport time on even large repositories to mere seconds, and closes #267.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link
Contributor

Only merged pull requests can be backported.

@winterqt
Copy link
Author

  1. This is a very trivial change, do I have to sign a CLA?
  2. Looks like the backport action fired overzealously, wonder why it ran, since this if is there...
    https://github.com/zeebe-io/backport-action/blob/7078123ca60e45c3452ca61d534dbd55c5319fec/.github/workflows/backport.yml#L16-L19

@github-actions
Copy link
Contributor

Only merged pull requests can be backported.

@winterqt
Copy link
Author

winterqt commented Jul 27, 2022

Never mind 🙃 got ahead of myself, didn't realize it failed, sorry.

2022-07-27T02:26:38.5164660Z Retrieve pull request data for #3
2022-07-27T02:26:38.7642213Z Check whether pull request 3 is merged
2022-07-27T02:26:38.8609497Z Detected labels on PR: backport backporttttttttt
2022-07-27T02:26:39.3721229Z From https://github.com/winterqt/nixpkgs
2022-07-27T02:26:39.3721889Z  * branch              3ccd08dd150c0ea0a926b15b28e6c19cb566f99d -> FETCH_HEAD
2022-07-27T02:26:39.8474263Z From https://github.com/winterqt/nixpkgs
2022-07-27T02:26:39.8477585Z  * branch              refs/pull/3/head -> FETCH_HEAD
2022-07-27T02:26:39.8505030Z Working on label backport backporttttttttt
2022-07-27T02:26:39.8505622Z Found target in label: backporttttttttt
2022-07-27T02:26:40.3655847Z From https://github.com/winterqt/nixpkgs
2022-07-27T02:26:40.3656660Z  * branch              backporttttttttt -> FETCH_HEAD
2022-07-27T02:26:40.3657304Z  * [new branch]        backporttttttttt -> origin/backporttttttttt
2022-07-27T02:26:40.3681610Z Start backport to backport-3-to-backporttttttttt
2022-07-27T02:26:40.4410594Z Error: 'git merge-base 3ccd08dd150c0ea0a926b15b28e6c19cb566f99d a8a11fa30582f4e1c0b4f0605744c40e8075bbe3' failed with exit code 1

@github-actions
Copy link
Contributor

Only merged pull requests can be backported.

@winterqt winterqt closed this Jul 27, 2022
@winterqt
Copy link
Author

Closing (for now).

@winterqt
Copy link
Author

Doesn't help that both stdout and stderr of git merge-base are empty when it's failing like this, added verbosity in winterqt@157b633 and that just logs this:

Error: 'git merge-base b8b02649c5abe7c27a96e726107fb2f664ef67ce e3e8c50689636952c55fbbfdaa75d30a64943c3f' failed with exit code 1 (stdout: , stderr: )

@github-actions
Copy link
Contributor

Only merged pull requests can be backported.

@korthout
Copy link
Owner

korthout commented Jul 27, 2022

Only merged pull requests can be backported.

Haha sorry about that. I've disabled the action for now, it's not really needed in this project anyways (I don't support multiple versions yet). Although it is strange that the action triggered on issue_comments that didn't themselves contain /backport.

@korthout
Copy link
Owner

do I have to sign a CLA?

At this time yes. The CLA is required because this repo is under zeebe-io, which is part of camunda. In the future, I may move the repo out of Camunda's orgs, but until then the CLA is still required.

Copy link
Owner

@korthout korthout left a comment

Choose a reason for hiding this comment

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

This will probably not work correctly in all cases, as not enough commits are fetched when the PR contains multiple commits or the target branch contains multiple commits since the PR was branched.

EDIT: 🤔 I'm not sure how to determine the number of commits to fetch on the base branch (PR target), perhaps we should simply fetch 1000 commits to start. That's already a factor 400 improvement for nixpkgs. We can further improve it if a need arises.

PS: thanks for giving it a try 🙇

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.

Speed regression when using fetch-depth = 1
3 participants