krux icon indicating copy to clipboard operation
krux copied to clipboard

[Followup] Add a hash step check on PR template

Open qlrd opened this issue 8 months ago • 4 comments

Describe your request

#577 updated the PR template by adding an important verification step to PR reviews: asking whether the developer built the firmware.

While this is very important, it lacks a "don't trust, verify" approach. That is, someone can say "yes, I built it". But how can we be sure? There is a way: provide the sha256sum hash of the build, so that it can be reproduced by reviewers.

This issue proposes some additional checks (worth changing):

### Did you build the code and tested on device?
- [ ] Yes
- [ ] No

### In which device you built?
- [ ] M5stickV
- [ ] Amigo
- [ ] Bit
- [ ] Docker
- [ ] Cube
- [ ] Yahboom
- [ ] WonderMV

Please provide the `sha256sum` hash of the build: <!-- Put here the hash -->

This may make the process more bureaucratic, but I believe it will also provide more certainty about what is being reviewed.

qlrd avatar Apr 19 '25 13:04 qlrd

Code hashes are useless in development because temporary changes and debug prints alter them. Requiring hashes per commit just creates busywork without real value. What matters is ensuring changes are properly tested, not matching hashes.

Also, the option "no" in the question it is redundant, if you did not checked "yes" it already implies a "no".

tadeubas avatar Apr 19 '25 17:04 tadeubas

List all devices to be checked could be useful only before a release, not development. For the template we could just change the "Yes" to Yes, build and tested on <!-- device-name -->

tadeubas avatar Apr 19 '25 17:04 tadeubas

List all devices to be checked could be useful only before a release, not development. For the template we could just change the "Yes" to Yes, build and tested on

Fixed

qlrd avatar Apr 20 '25 13:04 qlrd

PR #580 is merged into develop. Can we close the issue?

tadeubas avatar Apr 30 '25 21:04 tadeubas