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

Add a workflow to synchronize refs from git/git to gitgitgadget/git #1

Merged
merged 1 commit into from
Aug 26, 2023

Conversation

dscho
Copy link
Member

@dscho dscho commented Aug 18, 2023

The idea is, of course, to act on the push webhook that dutifully pings GitGitGadget's GitHub App every time a ref is pushed to git/git, and to let the App trigger this shiny new workflow to do the honors of keeping gitgitgadget/git up to date.

That change to the GitHub App will have to be implemented, still, but first this here PR needs to be reviewed and merged because GitHub workflows can only be triggered by workflow_dispatch events if their definitions are on the main branch.

@dscho dscho requested a review from webstech August 18, 2023 15:25
@dscho dscho self-assigned this Aug 18, 2023
@webstech
Copy link
Collaborator

A couple of other thoughts.

  • Should the source/target repos be allowed as inputs? Thinking of being able to test with any repo. That may be a problem with the auth stuff so is there anything there that could use a PAT if it was supplied?
  • If this was to be a model for another user of ggg, are there other things that need to be more flexible? Repo only has main branch is one consideration. Maybe not all that important until someone wants to use it as a model.

@dscho
Copy link
Member Author

dscho commented Aug 21, 2023

  • Should the source/target repos be allowed as inputs? Thinking of being able to test with any repo. That may be a problem with the auth stuff so is there anything there that could use a PAT if it was supplied?

I was considering this, but it looks as if this would rather complicate the code, and it is already more complicated than I had wished.

  • If this was to be a model for another user of ggg, are there other things that need to be more flexible? Repo only has main branch is one consideration. Maybe not all that important until someone wants to use it as a model.

Valid concern! However, this is something we need to tackle on the GitHub App side, which is expected to trigger this workflow.

@dscho
Copy link
Member Author

dscho commented Aug 21, 2023

@webstech thank you for your review! I pushed an updated version (addressing all suggestions except the reusable workflows one).

# fetch some commits
git fetch --depth 10000 source ${{ steps.check.outputs.source-sha }}
rm -f .git/shallow
fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't the rest of the code be included in this if? ie, no fetch, so no push?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@dscho Did you miss this or was I mistaken?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, sorry, I missed your comment.

No, the rest of the code still needs to run: the workflow could be triggered by a deletion, in which case source-sha would be empty.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the workflow could be triggered by a deletion, in which case source-sha would be empty.

TIL on a push, an empty <src> in the refspec will delete the <dst> ref. Good to know.

@webstech
Copy link
Collaborator

  • Should the source/target repos be allowed as inputs? Thinking of being able to test with any repo. That may be a problem with the auth stuff so is there anything there that could use a PAT if it was supplied?

I was considering this, but it looks as if this would rather complicate the code, and it is already more complicated than I had wished.

Wouldn't it be the same as refs (with more inputs specified)?

  SOURCE_REPOSITORY: ${{ inputs.source || 'git/git' }}
  TARGET_REPOSITORY: ${{ inputs.target || 'gitgitgadget/git' }}

Still thinking about how to have test runs against my own mock repos. It can wait if you want since the PAT support would also be needed.

@dscho
Copy link
Member Author

dscho commented Aug 22, 2023

I guess we could make those workflow inputs (with the current values in the env block via || "yaddayadda"), and then you can simply temporarily install the gitgitgadget app on your repository for testing... (you'd also need to fork -workflows and push this PR branch to your main because workflow_dispatch only works if it exists on the main branch.)

The idea is, of course, to act on the `push` webhook that dutifully
pings GitGitGadget's GitHub App every time a ref is pushed to `git/git`,
and to let the App trigger this shiny new workflow to do the honors of
keeping gitgitgadget/git up to date.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member Author

dscho commented Aug 22, 2023

we could make those workflow inputs

@webstech I've now done that.

@webstech webstech merged commit 6abbef2 into gitgitgadget:main Aug 26, 2023
dscho added a commit to gitgitgadget/gitgitgadget-github-app that referenced this pull request Aug 31, 2023
We added the `sync-ref` GitHub workflow in
gitgitgadget/gitgitgadget-workflows#1
specifically to let it be triggered by `push` webhook events delivered
to GitGitGadget's GitHub App. And this is the commit that teaches the
GitHub App that trick.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to gitgitgadget/gitgitgadget-github-app that referenced this pull request Sep 12, 2023
We added the `sync-ref` GitHub workflow in
gitgitgadget/gitgitgadget-workflows#1
specifically to let it be triggered by `push` webhook events delivered
to GitGitGadget's GitHub App. And this is the commit that teaches the
GitHub App that trick.

Signed-off-by: Johannes Schindelin <[email protected]>
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