fix(client/android): fix android network receiver while still supporting android 10

[daniellacosse]

IMG_2398.MOV

[fortuna]

Please make sure the client is working after the changes

[fortuna]

Please add comments to the code explaining why we need them

[daniellacosse]

Please make sure the client is working after the changes

Yeah, sorry, I should have made this a draft

[sbruens]

Please add comments to the code explaining why we need them

Could you still do this? It will be helpful for future readers to know what settings are important and why, so they don't change them.

[sbruens]

See my comment on adding some code comments where appropriate. Otherwise this LGTM, assuming it works on the various Android versions.

[daniellacosse]

Please add comments to the code explaining why we need them

Could you still do this? It will be helpful for future readers to know what settings are important and why, so they don't change them.

Generally my principle with inline code comments is to annotate things that aren't really behaving as intended (hacks and things like that). Comments get old and often lie, so there's an inherent risk in using them. If we comment this, the threshold for adding comments now becomes "anything that someone is likely not skilled enough in" which in my experience is fairly annoying to those who are literate in the space. Imagine for example commenting "sets the width to 100%" on the css property width: 100% every time (which may actually not always be the case if the element's display property is set to inline for instance). Comments like that on every other line ultimately pollute the source with "eventually false" information, imo.

I think as a compromise I can simply link to the relevant documentation in the comment, rather than write a paragraph attempting to explain how android works to the reader. The hope there is the documentation stays up to date. Is that fair?

[sbruens]

Imagine for example commenting "sets the width to 100%" on the css property width: 100% every time (which may actually not always be the case if the element's display property is set to inline for instance). Comments like that on every other line ultimately pollute the source with "eventually false" information, imo.

To be fair, that example is not quite the same. I'm not suggesting you comment what the code is doing, but why it's needed (see http://go/comments#reasons).

I think as a compromise I can simply link to the relevant documentation in the comment, rather than write a paragraph attempting to explain how android works to the reader. The hope there is the documentation stays up to date. Is that fair?

Thanks, yeah that's totally fine. I'm not expecting a liberally commented PR. You're just making very specific changes that are clearly non-trivial, but they are not self-describing. So to avoid future readers needing to dig it will be helpful context on why these specific settings are needed, or what the magic number means, so that someone doesn't inevitable try to remove it, especially because we don't have a regression tests for this.

[daniellacosse]

Imagine for example commenting "sets the width to 100%" on the css property width: 100% every time (which may actually not always be the case if the element's display property is set to inline for instance). Comments like that on every other line ultimately pollute the source with "eventually false" information, imo.

To be fair, that example is not quite the same. I'm not suggesting you comment what the code is doing, but why it's needed (see http://go/comments#reasons).

I think as a compromise I can simply link to the relevant documentation in the comment, rather than write a paragraph attempting to explain how android works to the reader. The hope there is the documentation stays up to date. Is that fair?

Thanks, yeah that's totally fine. I'm not expecting a liberally commented PR. You're just making very specific changes that are clearly non-trivial, but they are not self-describing. So to avoid future readers needing to dig it will be helpful context on why these specific settings are needed, or what the magic number means, so that someone doesn't inevitable try to remove it, especially because we don't have a regression tests for this.

Done!