Hacker News new | ask | show | jobs
by dewey 1506 days ago
> For science you can see the reactions here.

That link seems to be broken: https://github.com/issues?q=is%3Aissue+author%3Acode-review-...

I was actually surprised to read that people would ignore or be annoyed by a bot raising a valid PR that can be easily merged after a quick glance. What would be the reason for that?

10 comments

Automated checking of potential bugs in f-strings is hard. There are lots of false positives. You can see some discussion around this kind of rule in pylint [0]. At the end of the day, the choice to run automated linting tools on a repo is up to the maintainers. Autogenerating PRs like this is incredibly noisy and comes off to me as a blatant advertisement for their "code review doctor" product.

[0] https://github.com/PyCQA/pylint/issues/5039

> Autogenerating PRs like this

The article specifically mentions that they were not auto-generated,

"It was also interesting to see the reaction from open source developers to unsolicited pull requests from what looks like a bot (really a bot found the problem and made the PR, but really a human developer at Code Review Doctor did triage the issue before the PR was raised)"

In reactions they conveniently left out "false positives we still hadn't weeded out". On top of that it can be annoying to have bots making trivial PRs in their own format when you've got a well defined process for it. Lastly it was basically spamming an ad link for the service at the end of the PR comment - even if the other issues didn't come up it's not always well received to do that.

Looking at 1 bot it doesn't sound bad, when you have everyones bot doing this kind of stuff it can quickly become more of a nuisance than a help.

You're assuming that the PR is valid, but a maintainer can't make that assumption. They have to do the thankless work to figure out the context and handle the fallout if they get it wrong. Let's look at who wins:

    * Small benefit to bot creator
    * Tiny benefit to project
    * Modest cost to maintainer
Waves of low-effort resume-padding commits are already a thing. Not a big problem, but bots clearly have the potential to multiply the small problem into a big problem.

I'm still open to the idea that bots could be a net win, because most projects really do have heaps of small simple mistakes lying around. I'm sympathetic to the maintainers though. They always seem to get the short end of the stick.

https://github.com/Qiskit/qiskit-terra/pull/7982

That guy was not happy. I do agree that it's basically advertising and that's annoying.

I would never tolerate ads in my commit history. That's ridiculous.

It's basically using open source repos as an advertising platform for their static-analysis bot.

If they want to offer services, they can reach out to the maintainers. This is different than a human opening a valid PR on a OS repo since the commit message includes an ad and now they're advertising on HN.

In our case OPs bot did not open a PR which could have been merged quickly, but filed an issue instead.
What I've found from doing similar types of changes.

1. It's hard to explain the impact to the application of the current problem. Thus it looks like a theoretical issue

2. Sometimes people rely on the bug for their code to work

3. Surprise work can be poorly received (ie: not the current priority)

In addition to what others have already said, my own random sampling now shows quite a high false positive rate.
I expect to see the entire gamut of possible reactions with a sufficient number of bot PRs. But in looking at 10 of them at random, I didn't find a single "negative response."

(I don't think ignoring it is invalid or wrong by any means, given there's so many reasons one might not engage in a timely manner, or at all, in the issues section or PRs. I don't monitor my repos issues because I just don't feel interested in supporting my code. Feel free to fork or ignore!)

Some negative reactions:

https://github.com/mitmproxy/mitmproxy/issues/5285

https://github.com/Qiskit/qiskit-terra/issues/7981

https://github.com/beetbox/beets/issues/4340

I do think those concerns are legitimate. (I also think more tooling is a good thing!)

Thank you for sharing these links!
I looked through all three. The first isn't really a complaint because the bot acted in good faith and found an error. In the second one they complained abiout a missing unsubscribe link (reasonable) and in the third one, the author should update their code so it doesn't create a variable named path, then a non-f-string that includes "{path}". I had to stare at the author's comment that it was a false positive for quite a bit to convince myself they were right.
I will point out that in the first two issues, the repo owners also edited the initial report with something along the lines of "removed ad".

I disagree that the first isn't a complaint -- the owner stated that this behavior isn't appreciated but decided to let it slide because the issue was valid.

In the third issue, the owner also explicitly stated: "I don't think bots posting unsolicited static analysis results are a good idea." I have no opinions on whether the code should be clearer, but that doesn't change the validity of the reaction.

The bot-account's (apparently human-written) reply of "you're very welcome" to the complaint in the third issue is downright dismissive of the problem and kinda passive aggressive. While it seems that the bot did good work overall, the human(s) handling edge cases need work.
We've blocked the bot after their script malfunctioned and they opened a second issue with exactly the same text (https://github.com/mitmproxy/mitmproxy/issues/5286).
I do think the question of "how should bots that do static analysis work" is an important one, but in the meantime, people are gonna bot and repo managers are gonna complain.
Because this is basically just PR spam
to me, well-intentioned systems wiht a high true positive rate and low false positive rate are welcome so long as they follow reasonable etiquette and norms, which this group seems to do.