search

backdrop-ops / contrib

Apply to join the contributed code developer team.
23 stars 16 forks source link

Documentation: clarify bug squad's role with potential project releases #792

Open laryn opened 2 months ago

laryn commented 2 months ago

There was some confusion in a recent Zulip thread about what role the Bug Squad could take with regard to releases on projects. See the thread below.

Existing wording on the Bug Squad page:

Our Responsibilities include:

Potential new additions

If you are not available to review and/or merge bug-fixes, the bug-squad is here to help. We will merge community-approved fixes on your behalf, but will not make releases without your participation, unless the current release causes fatal or site-breaking errors, or if it has been more than a year since the last release.

The intent of the bug squad includes:

Below is the original thread:

@jenlampton

Reminder that the big squad is NOT authorized to make releases.

The only people who can make releases are maintainers and the security team, but the PRs merged CAN add new maintainers if a release is needed.

@laryn

I think that's different than what's listed here: https://backdropcms.org/leadership/bug-squad

...Creating a new releases [sic] when a sufficiently severe bug has been fixed

@argiepiano

I was under the impression that the bug squad also made releases (according to the docs linked above). I think the bug fixes I merged may not have merited the "severity" test, though, and for that, I apologize.

@yorkshire-pudding

I would have interpreted sufficiently severe to include fatal errors in use (not just on install), or a bug that stops it working as intended and cannot be easily worked around.

@jenlampton

This gets into a gray area. there are an infinite number of bugs (all of them?) that can be classified as "or a bug that stops it working as intended" - I think the real problem we are trying to prevent is if a project stops the rest of your Backdrop site from working as intended. A fatal error would qualify :)

klonos commented 2 months ago

Would things like settings/configuration not being saved properly qualify?

klonos commented 2 months ago

I was under the impression that the bug squad also made releases (according to the docs linked above).

Ditto.

jenlampton commented 2 months ago

Would things like settings/configuration not being saved properly qualify?

Only if if caused a fatal error.

yorkshire-pudding commented 2 months ago

I think the real problem we are trying to prevent is if a project stops the rest of your Backdrop site from working as intended.

Is it limited to that though? If someone is trying out Backdrop as a demo and they install a module they need (which will always be the latest release; they have no option to download the dev branch) and it doesn't work, but the fix is sitting there, merged but not yet released, are we not unnecessarily giving a bad first impression?

jenlampton commented 2 months ago

@yorkshire-pudding Any given module could have 100 edge-case bugs that could "cause a bad first impression" if someone tried to use it in a specific way that was untested (or unintended). The job of the bug squad isn't to fix all the bugs (in spite of the name!) -- It's primarily to keep people safe.

The reason the bug-squad isn't authorized to do all the work for existing maintainers is because we are concerned about the current maintainers being upset that they have lost control of their own project, thus making them less likely to want to maintain this, or any other project.

By allowing the bug squad to merge PRs but not make the release, we are providing the maintainers an opportunity to review the changes (and perhaps make their own modifications) before the release comes out. Thus catching any unintended consequences, or changes in intent of the project before the release.

Our community is better than most, but developers (myself included!) do have a tendency to get carried away, and this was our way of protecting the intent of the original authors.

jenlampton commented 2 months ago

Having the PRs merged (but not in a release) does still allow people using the module to download a zip from GitHub, and upload the latest code to their site if they are in need of the fix. This solution is for the people who are aware of the problem, are already in the GitHub queue, and can grab that zip easily.

The Releases issued by the Bug Squad are intended for people who are not aware of any problem, and could break their sites by installing (or updating to) the recommended version of a module.

I have included a proposed re-wording on the last bullet in the Original Post, but let's iterate as needed. Here's my starting point:

Creating a new release only when a bug that is severe enough to cause a fatal error or site crash has been resolved.

By allowing the bug squad to merge PRs but not make the release, we are providing the maintainers an opportunity to review the changes (and perhaps make their own modifications) before the release comes out. Thus catching any unintended consequences, or changes in intent of the project before the release.

Is it worth including this wording (or something similar) on the documentation page?

The use-case we were solving for is a maintainer who has limited time, or comes and goes. We want them to feel supported (but not replaced).

jenlampton commented 2 months ago

Or maybe we should specifically include abandoned projects in the bullet too?

Creating a new release only when a project is abandoned, or when a sufficiently severe bug has been fixed. (a "sufficiently severe bug" is one that can cause a fatal error or site crash during install or regular use.

laryn commented 2 months ago

I like that last suggestion -- bug squad making a release on an abandoned or unmaintained project will not be stepping on anyone's toes or preventing a non-existent maintainer from reviewing.

herbdool commented 2 months ago

After all this time, I still don't know how the decision is made on the Bug Squad policy changes or other policies. Who votes?

I don't think we need to define "sufficiently severe bug". We need some flexibility in interpreting. If people insist on defining the term then what has been offered so far is too restricted IMO. If a module doesn't operate as advertised then I consider that severe. For example, if the config doesn't save. Maybe they ported the module but made some mistakes so it doesn't actually work. Will it make the site crash? Probably not, but the module is unusable without a release.

jenlampton commented 2 months ago

After all this time, I still don't know how the decision is made on the Bug Squad policy changes or other policies. Who votes?

I don't think we vote, I think we discuss like we are now :) As long as everyone understands the previous intent (owner protection) and the current need (more fixes more quickly?), we can probably come out with something that we can agree on.

Will it make the site crash? Probably not, but the module is unusable without a release.

True. But if the module was ported by a beginner who wants to learn, and an expert comes in and fixes it for them it could cause the beginner not to want to work on the module (or any module) ever again. Is the potential of loosing that maintainer more important than a module that's never worked properly?

We don't want every PR merged by the bug-squad to trigger a new release. But if everyone is feeling good about having the bug squad do more releases, maybe the previous concerns about maintainers wanting individual control is too focused on how things were in Drupal, and not a reflection of how our more flexible Backdrop maintainers operate?

If people insist on defining the term then what has been offered so far is too restricted IMO.

I think the definition is helpful, but maybe we could add some exceptions, like the following, that allow us flexibility when we need it.

Does anyone have other suggestions?

herbdool commented 2 months ago

I think it's better to be explicit about the intent, instead of trying to define the term and try and account for exceptions.

"The intent is to encourage maintainers to be actively fixing their modules rather than having the Bug Squad fix them."

--

The problem we come across are maintainers who have clearly abandoned all their modules but we can't immediately find new maintainers. I don't think we have a policy for just removing the maintainer, which would give the Bug Squad more freedom in fixing things until a new maintainer comes along.

So I'd prefer not restrict the bug squad from at least keeping those modules working.

klonos commented 2 months ago

If a module doesn't operate as advertised then I consider that severe. For example, if the config doesn't save.

I'm on the same boat as @herbdool on this one ^^

But if everyone is feeling good about having the bug squad do more releases, maybe the previous concerns about maintainers wanting individual control is too focused on how things were in Drupal, and not a reflection of how our more flexible Backdrop maintainers operate?

Yes. That ^^ ...no new features. No UX/UI improvements. But bugs that render the module unusable, with an RTBC fix waiting in the queue for weeks/months should not really wait I think.

The intent is to encourage maintainers to be actively fixing their modules rather than having the Bug Squad fix them.

That sounds good to me 👍🏼 ...I would add something along the lines of this too:

At the same time, we are looking for ways to prevent verified fixes to severe problems from remaining unreleased for unreasonable amount of time.

(others usually have better words than mine, so please wordsmith the above as needed)

jenlampton commented 2 months ago

I don't think we have a policy for just removing the maintainer, which would give the Bug Squad more freedom in fixing things until a new maintainer comes along.

We do have one :) https://docs.backdropcms.org/documentation/abandoned-projects

I think it's better to be explicit about the intent, instead of trying to define the term and try and account for exceptions.

I like being explicit about the intent. Let's just do that and maybe remove the term "sufficiently severe bug" entirely? Here are some of our intentions with the bug squad:

The intent is to encourage maintainers to be actively fixing their modules rather than having the Bug Squad fix them.

This specific language sounds overly harsh. It reads like "Do your own work. Don't expect us to do it for you." I fear this would be discouraging to anyone who wants to be a maintainer.

What about stating something like: "If you are not available to review and/or merge bug-fixes, the bug-squad is here to help. We will merge community-approved fixes on your behalf, but will not make releases without your participation, unless [x], [y] and/or [z]."

But bugs that render the module unusable, with an RTBC fix waiting in the queue for weeks/months should not really wait I think.

We all agree that it should be merged so that anyone who needs the fix can get it if they need it. The question is whether we create a new release or not, and under what circumstances it's okay to do so. :)

The Bug Squad will need some guidance as to what they are and are not allowed to do, so regardless of how we phrase the documentation, we do need to decide when it's okay to make a release.

It sounds like we have agreed on one of the conditions being "If it's been more than a year since the last release", but we really should decide on the others. Any thoughts on these?

I do think it's important that we still allow a release to be created if it fixes a bug that causes a fatal error or site crash, even if some of the other requirements have not been met. So maybe that one should be an "or" instead of an "and". Should they all be "or"?

yorkshire-pudding commented 2 months ago

With regards to intent:

I like being explicit about the intent. Let's just do that and maybe remove the term "sufficiently severe bug" entirely? Here are some of our intentions with the bug squad:

  • decrease barriers to people becoming maintainers
  • grow the number of maintainers, specifically first-time or beginner maintainers
  • encourage all maintainers to keep working on their own projects along with the bug-squad
  • prevent any project from breaking a site when installed or updated via the installer
  • prevent verified fixes from remaining unreleased for unreasonable amount of time
  • provide bug-fixes and releases for abandoned projects
  • provide downloadable versions containing pre-release fixes (a.k.a. we merge PRs)

This looks good. I think another one could be to encourage new contributors who submit PRs to modules, get them tested but might otherwise get disheartened and discouraged from doing this again if the maintainer isn't even providing a holding response about merging and releasing.

Sometimes we do get modules that stop working through no fault of the maintainer. Not all changes in core consider the impact on modules (this can be tricky, but a bit more care could be taken), and other modules the module depends on could introduce breaking changes that stop the module working (when I took over Block reference, I had this issue).

What about stating something like: "If you are not available to review and/or merge bug-fixes, the bug-squad is here to help. We will merge community-approved fixes on your behalf, but will not make releases without your participation, unless [x], [y] and/or [z]."

I like this. I prefer "or" to making lots of "and" conditions.

jenlampton commented 2 months ago

I think another one could be to encourage new contributors who submit PRs to modules, get them tested but might otherwise get disheartened and discouraged from doing this again if the maintainer isn't even providing a holding response about merging and releasing.

I love it! I have shortened this to "encourage contributors who submit PRs with timely feedback and incorporation of the fix" and updated the Original Post.