-
Notifications
You must be signed in to change notification settings - Fork 190
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
Start september/october maintainer minutes #2819
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for nf-core-main-site ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
@nf-core-bot fix linting |
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
It was raised that regularly get almost-complete module PRs, but sometimes they stall because a reviewer leaves comments but does not come back to re-review. | ||
Even if the module author receives a second approving review, but the author often (because we are nice here at nf-core) doesn't feel comfortable merging without getting the approval from the original reviewer. | ||
|
||
After a short discussion we felt that we can add a new reviewing guideline that if all (reasonable) comments have been addressed, and an approval has been given by another reviewer, a PR should be merged after 3 months if the original reviewer did not re-review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a short discussion we felt that we can add a new reviewing guideline that if all (reasonable) comments have been addressed, and an approval has been given by another reviewer, a PR should be merged after 3 months if the original reviewer did not re-review. | |
After a brief discussion, we agreed to introduce a new reviewing guideline: if all reasonable comments have been addressed and another reviewer has approved, the PR should be merged after 3 months if the original reviewer has not returned for a re-review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im thinking maybe 3 months is too long ?? how about 1 month instead ? as I fear if this a nf-core beginner adopter and if they have to report back to their work that can't get it done for another 3 months they might lose interest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also wasn't the problem her the author not the reviewer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well if an author delays, thats their issue I guess. I think the point here is if nf-core reviewers delay the complete review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the discussion arose around nf-core/modules#5706, where the module maintainer didn'didn't come back to a PR changing things in their module.
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
sites/main-site/src/content/blog/2024/maintainers-minutes-2024-10-25.mdx
Outdated
Show resolved
Hide resolved
@nf-core-bot fix linting |
@maxulysse I've added some empty places for you to edit :)
@netlify /blog/2024/maintainers-minutes-2024-10-25