Feature Request: Spack bot should not ping all maintainers for simple version PRs

[robertu94]

It would be nice If a user is a maintainer for a patch which only updates the version number on a spack package, spack-bot would not ping the other reviewers for a review. Maybe it makes sense to ping the other maintainers if variants are modified, or some other more drastic change is made, but in my opinion, it would improve the signal to noise ratio coming from the bot.

In chatting about this on slack, there were some other possibilities suggested:

• have spack bot ask maintainers if another maintainer should review the patch when a maintainer submits the PR
• provide some way to configure which messages spack-bot will send on a per-package basis

[vsoch]

Thanks for opening this @robertu94 ! I'll keep a lookout for other cases like this so we can get more feedback.

[tgamblin]

Also:

• allow preferences to be expressed (maybe just via @spackbot commands) in the PR description.

[vsoch]

Yeah I kind of like the idea of not pinging anyone given that a maintainer has opened the PR for a package he/she maintainers, and then spackbot can be interacted with by the maintainer for some additional preference. @adamjstewart what do you think?

[tgamblin]

My only worry about this is that it's a somewhat subjective special case. I think we don't know what different maintainers want to be pinged about. That said, I can see the rationale for not pinging maintainers by default if the PR submitter is a maintainer, but I can also see where maintainers who do want to know about updates would want to know about updates from other maintainers as well. What if they disagree? Or what if a particular version is bad and someone does not know it? We've had issues like that with OpenMPI releases in the past.

I guess it's easy enough to know if a PR is a "single-line change that only updates the version number", but even that can be contentious/subjective. If the maintainer is, say, adding a line preferring an old version when none was preferred before, that's a single-line change that only touches a version, but that is also probably done for a reason that maintainers would want to know about.

I guess these points have me leaning towards something like a dictionary of notification preferences for maintainers, but that seems like a lot of complexity to add to packages. e.g.:

maintainers = {
    "robertu94": ["laundry", "dishwashing", "package hygiene", "real work"],
    "adamjstewart": ["versions"],
}

We'd have to label the change types somehow, and there are probably multiple dimensions here -- e.g., do you want to be pinged. We'd also have to validate this stuff in CI because I guarantee you maintainers aren't going to get all the details right in the list.

I think something simpler is probably better.

[vsoch]

@tgamblin that is what I was alluding to in slack, but then my immediate thought was "that's too complex!" but a cool idea nonetheless.

[adamjstewart]

I would disagree with this. New versions are likely to introduce bugs if the contributor forgets to update the dependencies. This happens extremely frequently for the packages I maintain.

[robertu94]

@adamjstewart the request here is about PRs made by maintainers of a package that has multiple maintainers. Not any potential contributor.

[adamjstewart]

I still think it's beneficial to at least let the other maintainers know. Especially for Python packages which may have maintainers who aren't familiar with how Python deps are listed. But I can understand why this is a gray area. I wouldn't be completely opposed, but I still like the current behavior.