Skip to content

Update codeowners - #305

Merged
dbiguenet merged 6 commits into
mainfrom
tmp/CN/Fix_CodeOwners
Aug 4, 2026
Merged

Update codeowners#305
dbiguenet merged 6 commits into
mainfrom
tmp/CN/Fix_CodeOwners

Conversation

@cnovel

@cnovel cnovel commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Mathieu, Rémi and Marc are now just mentionned whenever we change reality-data-client

Mathieu, Rémi and Marc are now just mentionned whenever we change reality-data-client
@cnovel

cnovel commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot Check if the new workflow works fine, without holes in its logic.

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@copilot Check if the new workflow works fine, without holes in its logic.

I reviewed and hardened the workflow logic in commit 46c3f32.

Key fixes:

  • switched to a safer pull_request trigger with explicit event types
  • added a fork guard so the job runs only for non-fork PRs
  • fixed permissions for PR issue comments (issues: write)
  • switched comment retrieval to pagination to avoid missing older comments
  • added null-safe comment body handling

I also ran validation checks (Code Review + CodeQL) after the updates.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

The new workflow has a confirmed async bug (missing await) and the CODEOWNERS/global ownership change appears to go beyond the PR description and needs confirmation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds an automation workflow to notify specific maintainers when typescript/packages/reality-data-client/ changes, and updates CODEOWNERS to reflect new ownership for that path (with maintainers now notified via workflow rather than code ownership).

Changes:

  • Add a GitHub Actions workflow that comments on PRs touching typescript/packages/reality-data-client/** to mention the maintainers.
  • Update .github/CODEOWNERS entries, including changing the owners for typescript/packages/reality-data-client/ and adjusting the global * rule.
File summaries
File Description
.github/workflows/mention-reality-data-client-changes.yml New workflow to comment and mention maintainers when reality-data-client files are changed.
.github/CODEOWNERS Updates ownership rules, including the reality-data-client path and the global catch-all rule.
Review details

Suppressed comments (2)

.github/workflows/mention-reality-data-client-changes.yml:65

  • github.rest.issues.createComment(...) returns a promise; without await, the script can finish before the API call completes, causing the mention comment to be skipped intermittently.
            github.rest.issues.createComment({

.github/workflows/mention-reality-data-client-changes.yml:37

  • This step says it gets “all comments on the PR”, but issues.listComments only returns PR issue comments (not review comments/reviews). If a maintainer is already mentioned in a review thread, this workflow will still ping them again.
            // Get all comments on the PR
            const comments = await github.paginate(github.rest.issues.listComments, {
              owner: context.repo.owner,
              repo: context.repo.repo,
              issue_number: context.issue.number
            });
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread .github/workflows/mention-reality-data-client-changes.yml
Comment thread .github/CODEOWNERS
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@dbiguenet
dbiguenet merged commit 762e8d9 into main Aug 4, 2026
4 checks passed
@dbiguenet
dbiguenet deleted the tmp/CN/Fix_CodeOwners branch August 4, 2026 07:26
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.

4 participants