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

update codeowners, to be more specific #7021

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

jxs
Copy link
Member

@jxs jxs commented Feb 21, 2025

Issue Addressed

I keep being notified for PR's like #7009 where it doesn't touch the specified directories in the CODEOWNERS file.
After reading the docs not having a forward slash in beginning of the path means:

In this example, @octocat owns any file in an apps directory
anywhere in your repository.

whereas with the slashes:

In this example, @doctocat owns any file in the /docs
directory in the root of your repository and any of its
subdirectories.

this update makes it more rigid for the files in the jxs ownership

@michaelsproul
Copy link
Member

I think the reason you were notified for 7009 is that we changed the target branch, which brought in everything from unstable to the PR, and then we rebased so the commits from unstable were no longer part of the PR

It might be better if we rebase first and then change the target branch, but github actions has a quirk where it won't re-trigger the target branch check when the target changes, meaning another rebase/dummy commit is required to get it to update

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Change looks to good to me regardless. I just don't think it will silence all the false positives because there are lots of PRs that either temporarily touch these files (due to merge commits/rebases) or which make very minor lint-level changes.

@michaelsproul michaelsproul added the ready-for-merge This PR is ready to merge. label Feb 23, 2025
Copy link

mergify bot commented Feb 23, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

@michaelsproul
Copy link
Member

@mergify requeue

Copy link

mergify bot commented Feb 24, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

mergify bot added a commit that referenced this pull request Feb 24, 2025
@mergify mergify bot merged commit 01df433 into sigp:unstable Feb 24, 2025
29 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants