So are we to merge #2226 or this PR - what would be better?

I've tested this:

• With branch contributors-update existing, the action runs successfully, however does not create a PR.

• With branch contributors-update no existing, the action fails.

I really appreciate your effort. But for now I prefer my fix, because it works 😉 I've already put much more time in this, as I wanted 😄

If you want to fix & test this PR, it's also fine for me 👍

With branch contributors-update existing, the action runs successfully, however does not create a PR.

create-pull-request has some conditions for if it should create a new PR or update an existing one. It's meant to be push to the branch if there's a valid diff between local and remote branches, and if so handle PR accordingly.

Looking at the workflow log, create-pull-request created a unique local branch, moved local changes over to that and did a force push to the remote branch. It then checked if it differed from master as the base branch and believed there was no diff so backed out of creating a PR :/

With branch contributors-update no existing, the action fails.

That's even more bizarre 🤔

Looking at the workflow log, it is the add-contributors action failing, which had some questionable documentation for understanding it's functionality :\

It appears to check the remote branch (that now no longer exists), to get the CONTRIBUTORS.md file contents to modify... which IIRC later uses PyGithub to push directly to the same branch it pulled from.. This is not great, and prevents the simpler workflow. It also might explain why create-pull-request behaved the way it did.

  File "/main.py", line 308, in main
    Contributors.get_data()
  File "/main.py", line 100, in get_data
    contents = self.repo.get_contents(self.PATH, self.BRANCH)
  File "/usr/local/lib/python3.9/site-packages/github/Repository.py", line 1789, in get_contents
    headers, data = self._requester.requestJsonAndCheck(
  File "/usr/local/lib/python3.9/site-packages/github/Requester.py", line 353, in requestJsonAndCheck
    return self.__check(
  File "/usr/local/lib/python3.9/site-packages/github/Requester.py", line 378, in __check
    raise self.__createException(status, responseHeaders, output)
github.GithubException.GithubException: 404 {"message": "No commit found for the ref contributors-update", "documentation_url": "https://docs.github.com/v3/repos/contents/"}

I'd need to contribute to that action at some point or find a replacement.

I really appreciate your effort. But for now I prefer my fix, because it works 😉 I've already put much more time in this, as I wanted 😄

If you want to fix & test this PR, it's also fine for me 👍

I understand, that's fair :)

I'll see if I can find time for it, at least for now we have something working.

for now we have something working.

I really hope so. We'll see in 30min 😄

Seems to work now:) @polarathene can we close this PR or is there more I overlooked?:)

This is a potentially better approach if I can find time in October to resolve the issues with add-contributors action (or replace it). Atm I don't have time to spare on this and do the necessary testing to verify the workflow in a working state. If I get around to it, I'll switch from draft status for review.

I'm not fond about our workflow using unmaintained actions (the branch management ones).

I don't mean to offend you, but we already have a few stale PRs, and from what you said, this PR is not even WIP. I would like to see this closed until it is worked on again. We can then leave it as a draft (after all, drafts are WIP not stale) and make it ready for review when finished. We don't have to close it, just my POV. What do you think?

I don't mean to offend you

Don't worry, I don't get offended easily 👍

but we already have a few stale PRs, and from what you said, this PR is not even WIP.

I'm not sure how this would be classed as stale? It's a change for a single workflow that will rarely be touched in the meantime. These changes proposed remain relevant, either I replace the add-contributors action or I submit a PR to that action to resolve the compatibility issues (eg it's local dev mode which allows doing a local file update instead of only remote update that the action is restricted to, I only need that toggle available to the action), and then it's a version bump on the action with the PR marked as ready for review again.

I would like to see this closed until it is worked on again.

Feel free to close it if you prefer that. I switched to draft as the PR itself is still relevant, I wouldn't create a new PR for it and the PR proposed here isn't exactly being rejected, draft status seemed valid and harmless.

If I don't find time in October to work on this, I could see how a PR in limbo might be unpleasant to have lingering around?

We can then leave it as a draft (after all, drafts are WIP not stale) and make it ready for review when finished. We don't have to close it, just my POV. What do you think?

I stand by my earlier response. I've made a time commitment to try resolve this before the end of October and this intent is conveyed with the switch to draft status as not being ready for review, but with intent to be ready in the near-future :)

It's very much fine with me, leave it open :) I was just curious.

I will tackle this for v13.0.0.

This (very likely) or I will close the PR 😆 (see #3289)

I am doubtful I'll ever get around to this with everything on my plate atm.

This was more QoL improvement and for maintainers. I wouldn't worry about it.

Repeating commit message here:

So I took this over and revised a change. The new workflow is much cleaner and uses non-archived, non-stale, up-to-date workflows.

An example PR (with an extra step to create a diff) can be found here. The corresponding runs are found here.

The new workflow uses only 3 other actions, and utilizes output from the create-PR-action to set the linting check to true (which works quite nicely). The styling was adjusted to reflect our current standards for GH Actions YAML files. The permissions were adjusted accordingly too, and the job now only runs once a week (more than enough, we needn't waste resources).

I think this looks good and is ready to be merged. Not sure how much you'd like to look into the changes @polarathene, or whether @casperklein want's to have a look as well before merging this. Just tell me :)

I'm now going to approving my own changes lol.

Not sure how much you'd like to look into the changes @polarathene

I don't think I can view what the original PR I put together here was since you've rewritten history? It's difficult to find a diff of before/after.

IIRC there was some issue prior, but you've presumably changed the workflow in some way and verified that it works? I can't provide a decent review right now, but trust that you've sorted out any concerns from before 👍

Not sure how much you'd like to look into the changes @polarathene

I don't think I can view what the original PR I put together here was since you've rewritten history? It's difficult to find a diff of before/after.

Sorry, I messed up because it was a first for me managing multiple upstreams with git. Hence I decided it was the cleanest solution to force-push this single commit.

IIRC there was some issue prior, but you've presumably changed the workflow in some way and verified that it works? I can't provide a decent review right now, but trust that you've sorted out any concerns from before 👍

I read the discussion and tested whether the workflow runs when a branch exists and when it does not exist. I cannot confidently (like 100%) tell whether BobAnkh/add-contributor is setup up perfectly, because I couldn't really test it (I didn't know how to synthetically add contributors), but everything else should be working nicely. I'm confident in going forward with this PR.

I will monitor the next execution to see whether it runs fine:)