-
-
Notifications
You must be signed in to change notification settings - Fork 104
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: fix undefined error and remove unused files #1325
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to make sure this new workflow can also be triggered manually:
workflow_dispatch:
inputs:
bot_comment_url:
description: |
Provide URL pointing to gitvote bot comment that contains closing voting update. It looks like `https:/asyncapi/community/issues/1313#issuecomment-2247595858`. We use this to update the voting summary in cases when we see errors in the voting status, when for example TSC member voted, but did a mistake and voted by adding emoji to main description or other bot comment instead of the correct way: which is adding an emoji to a comment from bot that opens the vote.
required: true
in your script you need to check if the workflow was run manualy, parse the URL and extract data and use GitHub API GET /repos/{owner}/{repo}/issues/comments/{comment_id}
to fetch the comment data and pass it to the script for later processing
most important changes that need to be done are here and regarding the package and package-lock that were pushed to master by mistake, you need to remove them with this PR |
.github/workflows/vote-tracker.yml
Outdated
runs-on: ubuntu-latest | ||
steps: | ||
|
||
- name: Checkout repository | ||
uses: actions/checkout@v4 | ||
|
||
- name: Installing Module | ||
run: npm install [email protected] | ||
run: npm install @octokit/rest [email protected] --no-save |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
run: npm install @octokit/rest [email protected] --no-save | |
run: npm install [email protected] --no-save |
it is not needed, you don't even use it
GH rest is part of github
context
.github/scripts/vote_tracker.js
Outdated
@@ -2,15 +2,23 @@ const yaml = require('js-yaml'); | |||
const { readFile, writeFile } = require('fs').promises; | |||
const path = require('path'); | |||
|
|||
module.exports = async ({ context }) => { | |||
module.exports = async ({ githuh, context, botCommentURL}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
github
not githuh
and once you do it, later instead of const { Octokit } = await import("@octokit/rest");
you can do github.request
github-script action comes with authenticated rest client -> https:/actions/github-script?tab=readme-ov-file. I think you used it in some other pr already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed Octokit used @derberg
.github/scripts/vote_tracker.js
Outdated
let message, eventNumber, eventTitle, orgName, repoName; | ||
|
||
if (botCommentURL) { | ||
await fetchCommentInformation() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is very risky approach and error prone long term, you basically do not know what happens without reading the code of fetch. I mean sharing variables like this, end mutating them across different functions
instead fetchCommentInformation
should return an object with all needed info, like orgName for example, and here you should assign returned values to variables
const voteCommentContext = await fetchCommentInformation();
orgName = voteCommentContext.orgName;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done @derberg
@derberg Need little bit testing more i will revert once done |
@asyncapi/bounty_team |
@derberg you can review now done with testing https:/AayushSaini101/Vote/blob/main/TSC_VOTING_OVERVIEW.md |
/rtm |
Resolves: #1324