web-features bot improvements for interop 2027 - #1318
Conversation
| # Trigger either when a single issue is opened or modified. | ||
| issues: | ||
| types: [opened, edited] | ||
| types: [opened, edited, labeled, reopened] |
There was a problem hiding this comment.
I assume because we might add a label that identifies something as a proposal? But maybe we should also have an if condition that checks whether the issue actually has the label before running further? https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#example-using-an-object-filter has an example of a condition that filters on labels.
| process-issue: | ||
| if: github.event_name == 'issues' | ||
| runs-on: ubuntu-latest | ||
| concurrency: |
There was a problem hiding this comment.
How many jobs might this trigger if we close issues en masse or add a lot of labels? GitHub Actions should just queue everything to eventually run, but if we know it'll be hundreds of jobs at the same time, that seems like it might break?
| run: npm ci | ||
| working-directory: scripts | ||
| - name: Run tests | ||
| run: npm test |
There was a problem hiding this comment.
Do we really need to run the tests when updating issues?
jgraham
left a comment
jgraham
left a comment
There was a problem hiding this comment.
Basically OK, although I'm quite confused with the comment creation flow.
In the future it would be really useful to separate out the changes into more commits; reviewing a mixture of minor formatting changes and multiple different feature changes is much harder.
| # Trigger either when a single issue is opened or modified. | ||
| issues: | ||
| types: [opened, edited] | ||
| types: [opened, edited, labeled, reopened] |
There was a problem hiding this comment.
I assume because we might add a label that identifies something as a proposal? But maybe we should also have an if condition that checks whether the issue actually has the label before running further? https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#example-using-an-object-filter has an example of a condition that filters on labels.
| "devDependencies": { | ||
| "octokit": "^5.0.3", | ||
| "web-features": "^2.48.0", | ||
| "web-features": "^3.34.2", |
There was a problem hiding this comment.
Should we also configure dependabot to automatically update this?
|
|
||
| if (!existingComment) { | ||
| console.log(`Posting a new comment on issue #${issueNumber}...`); | ||
| const response = await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/comments", { |
There was a problem hiding this comment.
Not sure why some GH requests are inline and others are functions.
| repository, | ||
| await listBotComments(octokit, repository, issueNumber), | ||
| ); | ||
| if (!commentToKeep) { |
There was a problem hiding this comment.
Not sure I understand. If this is undefined then we didn't find any comments. It seems like that depends on us not finding the comment we just created? But surely that's not something we can reliably depend on?
| return "created"; | ||
| } | ||
| if (commentToKeep.id !== response.data.id) { | ||
| console.log(`Another run created comment #${commentToKeep.id}; kept that comment instead.`); |
There was a problem hiding this comment.
That's only true inside the if I think.
| octokit, | ||
| repository, | ||
| fetchImpl = fetch, | ||
| featureCatalog = webFeatures, |
There was a problem hiding this comment.
I don't love this pattern where we're passing this through all the functions but also always providing a default. Let's just pass it through everywhere.
|
|
||
| // Handle moved and split features by redirecting to the target(s) in the catalog. | ||
| const processedFeatureIds = []; | ||
| for (const id of featureIds) { |
There was a problem hiding this comment.
This loop could end up with duplicates; maybe we want processedFeatureIds to be a Set instead?
3 participants
Fixes #1317.
Sorry that's a large PR with a lot of code changes. Hopefully, the summary above is helpful.
For what it's worth, the same code also lives at https://github.com/captainbrosset/interop-proposals-bot and I tested it by opening a few issues on that repo. If you want to test the new action/script more, feel free to open and edit issues on my personal repo.