Move firmware_version to protocol/#74
Conversation
|
Meta-nitpick: The commits in a pull request should either be squashed (ie, "fix meson build"), or else each commit should complete and consistent in itself. I suspect this PR should be just one commit. |
@rkr35 mentioned that this repository uses squash-then-merge strategy for merging PR changes. So I thought it was okay to have multiple commits when PR is under review (and I followed the same approach for my PRs since I prefer not to use force push). |
|
Both strategies are enabled -- I mildly prefer to rebase-then-merge, as it results in slightly cleaner history [1], and authors can arrange their commit sequence exactly as it should appear in the history. [1] Example: 751413a for PR #73 shows part of the commit message for "fix meson project", which is meaningless after the PR is reviewed and merged. |
I've always preferred squash-then-merge. In my person opinion, if you feel the need to have multiple commits included in upstream they should probably be separate independently reviewed PRs. For that "fix meson project", I should have updated the merge commit to remove it but I overlooked that. I generally do not like squashing then force-pushing to public branches after review as started since it messes with the history and make following changes to a PR as its reviewed more difficult. |
19e64e8 to
1c6132c
Compare
1c6132c to
2e23847
Compare
|
Fwiw, I'm more concerned about the readability of the git commit history (which we'll refer to in the distant future), than the convenience of review in github's review tool (which does not handle force-push gracefully). An argument for another time. |
This PR de-couples the EC_CMD_GET_VERSION wrapper from htool and moves it to protocol/. It also adds a setup adding mocks to libhoth_device for testing protocols.