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

Developed action #1

Merged
merged 14 commits into from
Apr 1, 2023
Merged

Developed action #1

merged 14 commits into from
Apr 1, 2023

Conversation

Bullrich
Copy link
Contributor

Created GitHub action that fetches all the stale issues.

Created documentation explaining how to use it.

@@ -0,0 +1 @@
* @paritytech/opstooling
Copy link
Contributor

Choose a reason for hiding this comment

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

Unknown owner on line 1: make sure the team @paritytech/opstooling exists, is publicly visible, and has write access to the repository

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that it is failing because it was set in the other organization. Should work once merged in ours

Copy link
Contributor

Choose a reason for hiding this comment

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

i guess the opstooling should have admin access in this repo and it should be ok

@Bullrich Bullrich requested a review from rzadp March 31, 2023 11:56
Copy link
Contributor Author

Choose a reason for hiding this comment

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

README.md Outdated Show resolved Hide resolved
@Bullrich Bullrich requested a review from rzadp March 31, 2023 12:08
@mordamax
Copy link
Contributor

mordamax commented Mar 31, 2023

few questions:

  • does it require all 10+ repo to duplicate the set up and a standard message? -> what would it take to slighly adjust a message in 10+ repos?
  • since there's an github app, which provides access to the issues, i don't fully understand why we would need a PAT if this includes private repos? afaik github app could be installed to the whole org, and have access to all repos including private, can we do it this way instead of PAT?
  • in the original task it states that these are "If someone from the community creates an issue there should be someone taking a look and reply." so then it provides a "lists GitHub issues without any reply that were created by non-team members.", which means that there should be a team provided too, right? which will be used in a query
  • updated_at is updated when someone comments the issue or adds a label?

@Bullrich
Copy link
Contributor Author

Bullrich commented Mar 31, 2023

  • does it require all 10+ repo to duplicate the set up and a standard message? -> what would it take to slighly adjust a message in 10+ repos?

No, we need one step per repo (so it can be fine tuned per repo).

In the example I added at the bottom of the readme, it handles 3 repos in the same action.

What this your action?

  • since there's an github app, which provides access to the issues, i don't fully understand why we would need a PAT if this includes private repos? afaik github app could be installed to the whole org, and have access to all repos including private, can we do it this way instead of PAT?

The reason for the PAT is for individual use. We can use pthe github app to generate the access token just like we do it in github-issue-sync

  • in the original task it states that these are "If someone from the community creates an issue there should be someone taking a look and reply." so then it provides a "lists GitHub issues without any reply that were created by non-team members.", which means that there should be a team provided too, right? which will be used in a query

I wanted to create that step in a separate commit. It also brings the question, does team refer to members of the organization?

  • updated_at is updated when someone comments the issue or adds a label?

Both.

@Bullrich
Copy link
Contributor Author

Bullrich commented Mar 31, 2023

updated_at is updated when someone comments the issue or adds a label?

@mordamax I created #2 for this.

Copy link
Contributor

@mordamax mordamax left a comment

Choose a reason for hiding this comment

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

LGTM, but certainly needs pagination for issues query, hope we can add it right after we merge this initial

README.md Outdated Show resolved Hide resolved
uses: slackapi/slack-github-action@v1.23.0
with:
channel-id: 'CHANNEL_ID,ANOTHER_CHANNEL_ID'
slack-message: "Stale issues this week: \n$LOCAL_ISSUES \n$ABC_ISSUES \n$XYZ_ISSUES"
Copy link
Contributor

Choose a reason for hiding this comment

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

could we make an example with all our repos in https://github.com/paritytech/opstooling/tree/master/.github/workflows ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, as soon as I have a Matrix bot I'll have it running!

I can make one with no Matrix bot but that will still work in the intended way.


Stale issues this week:
### Repo example/local has 1 stale issues
- [Stop AI from controlling the world](https://github.com/example/local/issues/15) - Stale for 25 days
Copy link
Contributor

Choose a reason for hiding this comment

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

😂

}

export const fetchIssues = async (octokit: InstanceType<typeof GitHub>, daysStale: number, repo: Repo): Promise<IssueData[]> => {
const issues = await octokit.rest.issues.listForRepo({ ...repo, per_page: 100, state: "open" });
Copy link
Contributor

@mordamax mordamax Mar 31, 2023

Choose a reason for hiding this comment

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

makes sense to add pagination
as at least this one https://github.com/paritytech/ink has > 100

may be as separate ticket would also be good to add filter by label.. so in such repos like substrate you could filter out the particular team tag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I'll add a pagination system in a different commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #4 to handle it

Co-authored-by: Maksym Hlukhovtsov <1177472+mordamax@users.noreply.github.com>
@Bullrich Bullrich merged commit 9e04807 into paritytech:main Apr 1, 2023
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.

None yet

3 participants