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

Update label action #1734

Merged
merged 3 commits into from
May 26, 2022
Merged

Update label action #1734

merged 3 commits into from
May 26, 2022

Conversation

PeculiarE
Copy link
Contributor

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/mapknitter-reviewers for help, in a comment below

@gitpod-io
Copy link

gitpod-io bot commented May 26, 2022

@codeclimate
Copy link

codeclimate bot commented May 26, 2022

Code Climate has analyzed commit 118f778 and detected 0 issues on this pull request.

View more on Code Climate.

@codecov
Copy link

codecov bot commented May 26, 2022

Codecov Report

Merging #1734 (eff394e) into main (ad65f85) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1734   +/-   ##
=======================================
  Coverage   63.47%   63.47%           
=======================================
  Files          39       39           
  Lines        1180     1180           
=======================================
  Hits          749      749           
  Misses        431      431           

@jywarren jywarren merged commit 9421a48 into publiclab:main May 26, 2022
@jywarren
Copy link
Member

Follow-up to 6fbf200 and 0e8aa81

@jywarren
Copy link
Member

jywarren commented May 27, 2022

OK, first i looked at the error log, it showed this:

Run hramos/label-actions@v1
  with:
    configuration-path: .github/label-actions.yml
    repo-token: ***
    perform: true
(node:1570) UnhandledPromiseRejectionWarning: TypeError: (s || "").replace is not a function
    at escapeData (/home/runner/work/_actions/hramos/label-actions/v1/node_modules/@actions/core/lib/command.js:66:10)
    at Command.toString (/home/runner/work/_actions/hramos/label-actions/v1/node_modules/@actions/core/lib/command.js:60:35)
    at issueCommand (/home/runner/work/_actions/hramos/label-actions/v1/node_modules/@actions/core/lib/command.js:[2](https://github.com/publiclab/mapknitter/runs/6625653792?check_suite_focus=true#step:3:2)[3](https://github.com/publiclab/mapknitter/runs/6625653792?check_suite_focus=true#step:3:3):30)
    at Object.issue (/home/runner/work/_actions/hramos/label-actions/v1/node_modules/@actions/core/lib/command.js:27:5)
    at Object.error (/home/runner/work/_actions/hramos/label-actions/v1/node_modules/@actions/core/lib/core.js:127:15)
    at run (/home/runner/work/_actions/hramos/label-actions/v1/lib/label-actions.js:[4](https://github.com/publiclab/mapknitter/runs/6625653792?check_suite_focus=true#step:3:4)8:10)
    at Object.<anonymous> (/home/runner/work/_actions/hramos/label-actions/v1/lib/label-actions.js:243:1)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
(node:1[5](https://github.com/publiclab/mapknitter/runs/6625653792?check_suite_focus=true#step:3:5)[7](https://github.com/publiclab/mapknitter/runs/6625653792?check_suite_focus=true#step:3:8)0) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:1570) [DEP001[8](https://github.com/publiclab/mapknitter/runs/6625653792?check_suite_focus=true#step:3:9)] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Then I updated the action to v2 (v1 was prob from the example)

Then i triggered it by adding the label to this PR, and got it to error, restarted the job with debugging enabled:

https://github.com/publiclab/mapknitter/runs/6625990189?check_suite_focus=true

##[debug]Evaluating condition for step: 'Process Label Action'
##[debug]Evaluating: success()
##[debug]Evaluating success:
##[debug]=> true
##[debug]Result: true
##[debug]Starting: Process Label Action
##[debug]Loading inputs
##[debug]Evaluating: secrets.GITHUB_TOKEN
##[debug]Evaluating Index:
##[debug]..Evaluating secrets:
##[debug]..=> Object
##[debug]..Evaluating String:
##[debug]..=> 'GITHUB_TOKEN'
##[debug]=> '***'
##[debug]Result: '***'
##[debug]Loading env
Run hramos/label-actions@v[2](https://github.com/publiclab/mapknitter/runs/6625990189?check_suite_focus=true#step:3:2)
  with:
    configuration-path: .github/label-actions.yml
    repo-token: ***
    perform: true
##[debug]Label added: dependencies
Error: TypeError: Cannot read property 'number' of undefined
Error: Cannot read property 'number' of undefined
##[debug]Node Action run completed with exit code 1
##[debug]Finishing: Process Label Action

@jywarren
Copy link
Member

I think it's probably this line!

https://github.com/hramos/respond-to-issue-based-on-label/blob/a366dfe725db739c4522391dd1cb3daa9cb399d5/src/respond-to-issue-based-on-label.js#L34

I wonder... is it not able to work on pull requests? Could it be adapted for pull requests? Hmm.

@jywarren
Copy link
Member

OK, I left a clarifying question on the repository for the action! hramos/respond-to-issue-based-on-label#12

@PeculiarE
Copy link
Contributor Author

PeculiarE commented May 28, 2022

Hi @jywarren,

I tried digging around myself and stumbled to the same conclusion: maybe the bot specifically caters to only issues, no PRs.

Here's a similar bot that does roughly the same thing but it's clearly stated in the docs that it handles both PRs, issues, and discussions: dessant/label-actions

I went ahead and explored their codebase and noticed that they had to differentiate between issues and PRs when accessing the webhook payload and writing their internal functions here and here.

I could be wrong but it appears that if the event is an issue, the payload details coming from Github is contained in the key 'issue'. However, if the event is triggered by a pull request, then the key is 'pull_request' instead. This further confirmed my suspicion that the one we settled on solely focused on issues only.

Alas, the all-encompassing bot is not without its own limitations. It has a major roadblock: if the creator of the PR is a bot, then the workflow won't run. At least, that's what I figured from this line in the codebase. That kinda defeats the whole point of our search, as the PRs to be commented on are created by dependabot 😞

Several hours of googling and gazillions of tabs later 😆, I figured we could do it ourselves using GitHub API and GitHub Actions. The latter runs the workflow and the former helps us make the comment ourselves provided the PR is made by dependabot.

Tested it out on my forked repo with over 10 PRs made by dependabot and each was auto-commented on as desired.

So, I went ahead to make a PR at #1738. Please help review 😄.

Thank you!

@jywarren
Copy link
Member

Ah, just caught up here. Wow, fantastic and deep work here! I hope it was fulfilling rather than overly frustrating! It's great to see it working. Now we can copy this into plots2! You might be interested at some point in releasing your own Github Action, too! I've never done that!

@jywarren
Copy link
Member

Just linking to the working PR where we see the comment now! #1741 (comment)

@PeculiarE
Copy link
Contributor Author

PeculiarE commented May 31, 2022

Ah, just caught up here. Wow, fantastic and deep work here! I hope it was fulfilling rather than overly frustrating! It's great to see it working.

Thank you ❤️ @jywarren.....there were hints of frustration here and there but nothing beats the feeling of finally resolving an issue. So, yeah, I'm feeling fulfilled 😄.

Now we can copy this into plots2!

Cool! Seems perfect for an FTO. Would we be adding this to only plots2? or all our repos as well?

You might be interested at some point in releasing your own Github Action, too! I've never done that!

Wow 😄 ...didn't think about this. Just only entertained the idea of possibly writing about my experience to help others who might have the same issue. But now that you've mentioned it, it's worth taking a shot at. Thank you!

@PeculiarE
Copy link
Contributor Author

Just linking to the working PR where we see the comment now! #1741 (comment)

Oh, I think this links to the wrong comment

@jywarren
Copy link
Member

Ah true it's #1741 (comment) -- great.

@jywarren
Copy link
Member

Oh and yes, i do think we could make an FTO of this!

PeculiarE added a commit to PeculiarE/mapknitter that referenced this pull request Jul 6, 2022
* chore: create workflow for processing label action

* Update label-actions.yml

* chore: add configuration path
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