4 ms·

> bug fix updates getting rejected for stupid, unrelated, nitpicky reasons, things that had already been in the app for months or years This reminds me of code

> bug fix updates getting rejected for stupid, unrelated, nitpicky reasons, things that had already been in the app for months or years This reminds me of code review in general. Some reviewers have the etiquette of "if it was there already, then it's outside the scope of this review," and some don't. Maybe a reviewer should have two responsibilities: 1. Review the proposed _change_ and verify that it meets all criteria and doesn't introduce defects. 2. Make note of any existing defects, which are then recorded. Maybe an app developer can be required to resolve everything found in (2) before the _next_ review. I've never released an app before.

eru 3y ago
> Some reviewers have the etiquette of "if it was there already, then it's outside the scope of this review," and some don't. > Maybe a reviewer should have two responsibilities: [...] For code review: you can make lots of different choices for responsibilities and still have a good code review process. It just needs to fit your goals and people in your organisation should be on the same page about it. So for example for code review, I don't think the reviewer should try very hard to understand the change. Explaining the change so that it is easy to understand is one of the responsibilities of the author who proposes the change. Similarly it's upon the author to demonstrate that the proposed change doesn't introduce any defects and to explain how it meets all criteria. It's not on the reviewer to go bug hunting. That is so that the next guy who view the change in a few years in version control history to understand how the software developed (or how a bug was introduced) has a fighting chance to understand what happened from what was preserved in version control alone.
joshspankit 3y ago
1. Since most changes are compiled binaries, you would have to rely on the submitter to self-report what changed.
- dvzk 3y ago
  Things open source developers say because they don’t have experience with binary analysis. Not to say that Apple’s reviewers are going to be looking at IDA or Ghidra dumps, but they can see added capabilities, frameworks, symbols, or assembly and call graph changes. Not that it matters, because they don’t have the time to manually review source code changes either. At best there’s likely some automated static analysis for undocumented API symbols and common malware signatures.
  - saagarjha 3y ago
    A larger issue is that to review these kinds of changes you need to have a baseline competency in understanding how code works, and that’s expensive to procure at the scale needed to do proper review.
  - realusername 3y ago
    A bad actor can just carefully exclude the reviewers from ever seeing those changes (and it's routinely done on a large scale).
saagarjha 3y ago
Code review is typically based on a premise that the person submitting the change is acting in good faith. This is the only reason it works, because otherwise it’s difficult to review them sufficiently. App review must consider developers to be potentially hostile.
- lapcat 3y ago
  > App review must consider developers to be potentially hostile. And vice versa. ;-)
  - saagarjha 3y ago
    Generally I’d plead incompetence but if you’re trying to compete with Apple I’ll accept your point ;)
achates 3y ago
Whenever I get a comment about a pre-existing bug in a code review I make a ticket to fix it and thank them for pointing it out, but I never fix it in the PR. It gets too messy for the other reviewers and too confusing for QA when you end up with a bunch of unrelated fixes in one update.

eru 3y ago

> Some reviewers have the etiquette of "if it was there already, then it's outside the scope of this review," and some don't. > Maybe a reviewer should have two responsibilities: [...] For code review: you can make lots of different choices for responsibilities and still have a good code review process. It just needs to fit your goals and people in your organisation should be on the same page about it. So for example for code review, I don't think the reviewer should try very hard to understand the change. Explaining the change so that it is easy to understand is one of the responsibilities of the author who proposes the change. Similarly it's upon the author to demonstrate that the proposed change doesn't introduce any defects and to explain how it meets all criteria. It's not on the reviewer to go bug hunting. That is so that the next guy who view the change in a few years in version control history to understand how the software developed (or how a bug was introduced) has a fighting chance to understand what happened from what was preserved in version control alone.

joshspankit 3y ago

1. Since most changes are compiled binaries, you would have to rely on the submitter to self-report what changed.

dvzk 3y ago
Things open source developers say because they don’t have experience with binary analysis. Not to say that Apple’s reviewers are going to be looking at IDA or Ghidra dumps, but they can see added capabilities, frameworks, symbols, or assembly and call graph changes. Not that it matters, because they don’t have the time to manually review source code changes either. At best there’s likely some automated static analysis for undocumented API symbols and common malware signatures.
- saagarjha 3y ago
  A larger issue is that to review these kinds of changes you need to have a baseline competency in understanding how code works, and that’s expensive to procure at the scale needed to do proper review.
- realusername 3y ago
  A bad actor can just carefully exclude the reviewers from ever seeing those changes (and it's routinely done on a large scale).

saagarjha 3y ago

Code review is typically based on a premise that the person submitting the change is acting in good faith. This is the only reason it works, because otherwise it’s difficult to review them sufficiently. App review must consider developers to be potentially hostile.

lapcat 3y ago
> App review must consider developers to be potentially hostile. And vice versa. ;-)
- saagarjha 3y ago
  Generally I’d plead incompetence but if you’re trying to compete with Apple I’ll accept your point ;)

achates 3y ago

Whenever I get a comment about a pre-existing bug in a code review I make a ticket to fix it and thank them for pointing it out, but I never fix it in the PR. It gets too messy for the other reviewers and too confusing for QA when you end up with a bunch of unrelated fixes in one update.