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

refactor: moving issue-assigning logic to make it re-usable #3284

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

olegbespalov
Copy link
Contributor

@olegbespalov olegbespalov commented Aug 16, 2023

What?

Moving issue assigner logic to make it re-usable.

Why?

Hopefully, that way, we could have a single source of the truth for the issue assigned code, and re-use this in our other repositories while waiting for the better implementation of the codeowners.

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make ci-like-lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

@olegbespalov olegbespalov added this to the v0.47.0 milestone Aug 17, 2023
@olegbespalov olegbespalov requested review from oleiade and removed request for codebien August 17, 2023 06:45
- uses: actions/github-script@v6
with:
script: |
const assignees = ['mstoykov', 'codebien', 'olegbespalov', 'oleiade'];
Copy link
Contributor

Choose a reason for hiding this comment

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

You might consider making it an input for this action to avoid changing the implementation whenever we need to add/remove a team member from this list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we will use this mainly as the default single source of the truth for most actions, I expect the input to be empty.

However, I don't mind implementing the custom input with stressing that it's still not the generic auto-assigner, but the k6's one and we have no promises to break it 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe let's first test that it works across repos and then we can start changing it?

But yeah - it will probably be nice if this reads the CodeOwners file ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generally, yeah it could have many more features, depending on our capacity and desire to maintain it, but as the first iteration, even the current version should cover our main pain, the issues discovery in other repos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the record, I've changed my mind, and we shouldn't provide input here but use a static list to keep this as the single source of truth and have no way to create the case where the list will be distributed and overrided.

Copy link
Contributor

Choose a reason for hiding this comment

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

Having this list fetched directly from the GitHub team's members would be great. Then we could also inspect the members' status and exclude the busy people (but we require https://github.com/orgs/community/discussions/42788).
Ofc, all of this after we have validated the current implementation, that sounds perfect as a starting point.

with:
script: |
const assignees = ['mstoykov', 'codebien', 'olegbespalov', 'oleiade'];
const assigneeCount = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this one could be an input too?

@codebien
Copy link
Contributor

@olegbespalov should we merge grafana/xk6-grpc#36 before for testing or would it not work?

@olegbespalov
Copy link
Contributor Author

Okay, as agreed internally, we first try this as the wrapper. Later we could change actually mechanism for the issue assigning it to anything more powerful.

@olegbespalov olegbespalov merged commit e21589c into master Aug 23, 2023
21 checks passed
@olegbespalov olegbespalov deleted the chore/absrtracting-issue-assigner branch August 23, 2023 13:43
@olegbespalov
Copy link
Contributor Author

It doesn't work, I'll rever this and go with the re-usable workflow instead of the action 😢

assignees: getNRandom(assigneeCount, assignees),
});
- name: Assign k6's maintainer to issue
uses: ./.github/actions/issue-assigner/
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you need to give repo@master/path ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants