4 ms·

As a maintainer for two less popular repos (3K & 5K stars), I'm very happy to get small PRs with incremental changes. Many of our large contributors have starte

As a maintainer for two less popular repos (3K & 5K stars), I'm very happy to get small PRs with incremental changes. Many of our large contributors have started this way. We have a robust test and linting suite on both, so we often don't need much human engagement for reviews — code looks reasonable, tests pass, smash "Merge". Out of interest — do you find they're not actually improving the code? Or it's just not worth the time to review it?

toimtoimtoim 4y ago
For example: In Go you have `x.Close()` / `err := x.Close()` that returns an error and if someone starts to check/assert those usages in tests which you have ignored in test code. I think they are pushing the limits. No - these do not add value. No new use-cases are being checked, no improvement to coverage etc. or typo check tests. I do not reject these PRs. I review these and merge but I feel annoyed. This non-sense started somewhere around start of 2022. Before that I rarely ever see PRs like that.
- robswc 4y ago
  Could it be people using CoPilot trying to score brownie points? lol
- maximilianroos 4y ago
  Them doing it and you getting annoyed sounds like a bad equilibrium! I'd echo the sibling comment re adding what is / isn't appreciated into a CONTRIBUTING.md. My guess is people are doing it to get their feet wet with the repo, being helpful on the margin, and would not do it if they thought it was unhelpful on the margin. Here's PRQL's: https://github.com/prql/prql/blob/main/CONTRIBUTING.md https://github.com/prql/prql/blob/main/CONTRIBUTING.md
robswc 4y ago
I don't maintain a repo of that size but I have to say this sounds more reasonable. I guess it totally depends on the project. I guess tons of PRs could be annoying _but_ if they are actually useful PRs, then it would be nice to have smaller, more manageable PRs, no?

toimtoimtoim 4y ago

For example: In Go you have `x.Close()` / `err := x.Close()` that returns an error and if someone starts to check/assert those usages in tests which you have ignored in test code. I think they are pushing the limits. No - these do not add value. No new use-cases are being checked, no improvement to coverage etc. or typo check tests. I do not reject these PRs. I review these and merge but I feel annoyed. This non-sense started somewhere around start of 2022. Before that I rarely ever see PRs like that.

robswc 4y ago
Could it be people using CoPilot trying to score brownie points? lol
maximilianroos 4y ago
Them doing it and you getting annoyed sounds like a bad equilibrium! I'd echo the sibling comment re adding what is / isn't appreciated into a CONTRIBUTING.md. My guess is people are doing it to get their feet wet with the repo, being helpful on the margin, and would not do it if they thought it was unhelpful on the margin. Here's PRQL's: https://github.com/prql/prql/blob/main/CONTRIBUTING.md https://github.com/prql/prql/blob/main/CONTRIBUTING.md

robswc 4y ago

I don't maintain a repo of that size but I have to say this sounds more reasonable. I guess it totally depends on the project. I guess tons of PRs could be annoying _but_ if they are actually useful PRs, then it would be nice to have smaller, more manageable PRs, no?