Skip to content

Conversation

@SashankBhamidi
Copy link
Member

@SashankBhamidi SashankBhamidi commented Oct 27, 2025

Contributes to #945

Default owners get requested on all PRs since we use *. Specific paths add more reviewers depending on what files changed.

~18 mins

@SashankBhamidi
Copy link
Member Author

Feel free to edit the file and add any areas you'd like to be automatically assigned to review.

Do not merge upon approval, everyone needs to agree to be added.

@coveralls
Copy link

coveralls commented Oct 27, 2025

Pull Request Test Coverage Report for Build 19026002763

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.871%

Totals Coverage Status
Change from base Build 19025215976: 0.0%
Covered Lines: 8154
Relevant Lines: 8323

💛 - Coveralls

@SashankBhamidi
Copy link
Member Author

This overrides the above, providing greater specificity, and therefore excludes me, which is what I want.

@stevepiercy Just to clarify, you'll still be requested as a reviewer on every PR since you're a default owner with *. The specific rules like /src/icalendar/tests/ add additional reviewers, they don't exclude you. So you'll see all PRs regardless of those more specific paths.

I categorized specific sections so others can be added as reviewers for only the parts of the codebase they're interested in, we can modify the file accordingly later if required (so I've added your suggestion).

@stevepiercy
Copy link
Member

This overrides the above, providing greater specificity, and therefore excludes me, which is what I want.

@stevepiercy Just to clarify, you'll still be requested as a reviewer on every PR since you're a default owner with *. The specific rules like /src/icalendar/tests/ add additional reviewers, they don't exclude you. So you'll see all PRs regardless of those more specific paths.

Maybe I misinterpreted this?

https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners#example-of-a-codeowners-file

# These owners will be the default owners for everything in
# the repo. Unless a later match takes precedence,
# @global-owner1 and @global-owner2 will be requested for
# review when someone opens a pull request.
*       @global-owner1 @global-owner2

# Order is important; the last matching pattern takes the most
# precedence. When someone opens a pull request that only
# modifies JS files, only @js-owner and not the global
# owner(s) will be requested for a review.
*.js    @js-owner #This is an inline comment.

I interpret it to mean that a later rule overrides earlier ones, so I would be excluded whenever a later rule does not include my username.

@SashankBhamidi
Copy link
Member Author

I interpret it to mean that a later rule overrides earlier ones, so I would be excluded whenever a later rule does not include my username.

Thanks for clarifying with the docs. In other orgs I'm part of, I've seen default owners still get requested even with specific rules, but they might have different settings or external apps that override the standard CODEOWNERS behavior.

Copy link
Member

@niccokunzmann niccokunzmann left a comment

Choose a reason for hiding this comment

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

Thanks for adding this. I added closing criteria to the issue. So, this still does not close it until documented. That can be done in a follow-up PR though.

@niccokunzmann
Copy link
Member

@stevepiercy ready to merge when you are.

@abe-101
Copy link
Collaborator

abe-101 commented Oct 27, 2025

You can a me to the documentation

Copy link
Member

@niccokunzmann niccokunzmann left a comment

Choose a reason for hiding this comment

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

Adding @abe-101

@niccokunzmann
Copy link
Member

@abe-101Thanks! I added you to most of the documentation files, the ones concerning content, not configuration.

@niccokunzmann niccokunzmann self-requested a review October 27, 2025 11:51
Copy link
Member

@niccokunzmann niccokunzmann left a comment

Choose a reason for hiding this comment

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

I added abe-101

@SashankBhamidi
Copy link
Member Author

Thanks for adding this. I added closing criteria to the issue. So, this still does not close it until documented. That can be done in a follow-up PR though.

@stevepiercy Do you think if you can work on this?

@stevepiercy
Copy link
Member

Please, let's give others time to respond so they don't have to chase down another PR, and to give me time to write the documentation. I've got higher priority $WORK and a personal matter to address first.

Copy link
Member

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Add @abe-101 to remaining documentation files and configuration.

@stevepiercy
Copy link
Member

@SashankBhamidi I assume you have write permission to this repo. In the future, can you create a new branch in this repo instead of your fork? That simplifies checking out a branch to work on it.

FYI, I'll work on docs by the end of the week. $WORK is in my way.

@stevepiercy
Copy link
Member

Docs are now written and pushed to this PR. One more review pass, and it's good to go! @SashankBhamidi @niccokunzmann

@stevepiercy
Copy link
Member

Note that I deliberately used the invalid link https://github.com/collective/icalendar/blob/main/.github/CODEOWNERS because it will exist after this PR is merged. Please ignore the error for this link only.

@stevepiercy
Copy link
Member

Actually, I'll just remove that link. It's not necessary, as most people will be using an IDE anyway.

@niccokunzmann
Copy link
Member

Thanks for the reviews and contributions!

@niccokunzmann niccokunzmann merged commit 4e79e3b into collective:main Nov 3, 2025
20 checks passed
@SashankBhamidi SashankBhamidi deleted the add-codeowners branch November 3, 2025 12:05
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.

6 participants