Tag app authors in PRs to their apps #3994
Conversation
|
tagging thyttan for |
bobrippling
changed the title
|
Nice! This looks really good! |
|
I'm wondering, does the field support multiple authors? |
Knew there was something I'd forgot - yes, thanks! |
|
Thanks! This looks good, and glad it's working now - as you say in the header it'd be nice if the author was higher up the JSON though. I see it says '54 files changed' but there are 600-odd apps? Or is it only searching recent history?
You could always separate them with commas? |
|
@gfwilliams as for the 54 files changed, @bobrippling manually added them, as far as I can tell. App authors will have to add their username to their apps in the future... |
Yeah they were manually added, the remaining apps would have to be either updated also in future or we could track down the first commit to that app an assume that's our author |
|
Maybe I'm being a bit of a luddite here, but would it make sense for us to not have a GitHub action, but just a script that we ran manually every so often that set the author? It'd be easier to get it to set the If the GitHub action doesn't run on PRs but only branches, presumably it means we couldn't rely on it to always update the author (if we get a PR from someone who doesn't have GitHub Actions?) I'm not entirely sure whether having something that modifies the contents or PRs for us is going to come back and bite us later? Like if I did a PR from my PC, then changed the metadata around where the author tag went in and went to push again, wouldn't I get a merge error? |
|
Hmm, we could make a script that just scans for the first commit author and sets the metadata author tag to that person, asa way to update all apps, but I like the idea of having a GitHub action for tagging the authors in every PR |
Ah I've perhaps confused things here - the setting of authers I've left as entirely manual for now, I just used a hacky script initially to populate a few apps I could be certain about. But I don't plan on automating that hacky script any time soon |
bobrippling
force-pushed
the
test/app-authors-tag
branch
from
a7f929e to
51da98e
Compare
|
tagging thyttan for |
bobrippling
force-pushed
the
test/app-authors-tag
branch
from
51da98e to
36f3321
Compare
and error handling for curl
bobrippling
force-pushed
the
test/app-authors-tag
branch
from
36f3321 to
5805732
Compare
|
Skipping PR |
|
Should now just get a tag for gfw 🤞 (for the changes I made to example apps) |
|
tagging gfwilliams for apps/_example_app, apps/_example_clkinfo, apps/_example_clock, apps/_example_widget |
bobrippling
force-pushed
the
test/app-authors-tag
branch
from
e6a5888 to
7894822
Compare
|
tagging gfwilliams for |
|
I'm happy with this now, will run a test PR after it's merged to try it out |
|
Ok, great - let's merge then! |
|
Thanks for your work on this! |
|
Thanks for the reviews & merge! |
As seen in #3932, it can take a bit of research to find the author of an app. This PR adds an author field so people can add themselves to be notified of PRs which might need their review.
I've also added a few people who I could programatically glean from the git history.
Doing this from a branch within the repo to avoid the permission problems in #3935. Going off #3996, it looks like this'll need to be merged to
masterbefore PRs will be able to work with it.authorfield to metadata example EspruinoDocs#753metadata.json- after versionauthor: string | string[]