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

[ci] [docs] add Oliver to CODEOWNERS #6247

Merged
merged 1 commit into from
Dec 22, 2023
Merged

[ci] [docs] add Oliver to CODEOWNERS #6247

merged 1 commit into from
Dec 22, 2023

Conversation

jameslamb
Copy link
Collaborator

Adds LightGBM's newest maintainer, @borchero , to CODEOWNERs.

Removes some unnecessary and out-of-date information from https://lightgbm.readthedocs.io/en/latest/FAQ.html.

@@ -7,4 +7,4 @@
# offer a reasonable automatic best-guess

# catch-all rule (this only gets matched if no rules below match)
* @guolinke @jameslamb @shiyu1994 @jmoralez
* @guolinke @jameslamb @shiyu1994 @jmoralez @borchero
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@borchero this will automatically add you as a reviewer on every PR.

To be sure we set the right expectation...you do not need to review every PR! We require 1 approval from a maintainer to merge. In some situations, like more controversial changes or changes where particular maintainers have specialized knowledge, we sometimes ask for multiple approvals.

For example, for any non-trivial changes to distributed training, I'll probably ask for approvals from both @jmoralez (Dask knowledge) and one of @shiyu1994 and @guolinke (C++ and LightGBM internals knowledge).

@@ -11,44 +11,15 @@ LightGBM FAQ

------

Critical Issues
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These changes are included in this PR because I originally came to this page to add @borchero to the list of maintainers.

Explanation of the changes to these docs:

  • none of this applies to only "Critical" issues, so removed language like that
  • list of maintainers was seriously out-of-date
    • many of these people have not interacted with the repo in years
    • some of the notes about specialization were wrong (e.g. implying that I only work on R and Dask)
  • example questions don't add much information above what we ask for in the issue templates (https://github.com/microsoft/LightGBM/tree/master/.github/ISSUE_TEMPLATE)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm basically proposing changing this from "list of all maintainers" to "who you should @ if you need timely help".

@jameslamb jameslamb merged commit 1bd3d7e into master Dec 22, 2023
41 checks passed
@jameslamb jameslamb deleted the add-maintainer branch December 22, 2023 15:21
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.

3 participants