[M] Change in flashrom[master]: tests: add basic lifecycle test for ch341a_spi
Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67664 ) Change subject: tests: add basic lifecycle test for ch341a_spi ...................................................................... Patch Set 6: (1 comment) File tests/libusb_wraps.c: PS5:
Why? In all previous tests, wraps were added in the same patch with the test that needed those wraps […] Commits should only do one thing and not multiple things. I think this commit does two things:
* Introducing lots of functions in common code * Introducing tests which are based on the new common code Speaking generally, I think it's a bad practice to submit a commit of this kind, because it's not atomic. The commit title doesn't even give me an idea that new common code is introduced, but many comments are related to the common code. Assuming that future commits get merged, which depend on the newly introduced common code, and we decide or we would like to revert this commit (for whatever reason, everything can happen), then we also have to revert all others that depend on it. Don't get me wrong. I'm not saying that this will be the case for this commit. I just would like to move to this practice, since it gives us more possibilities in the future when we try or even have to fix something. Like a revert might be faster and easier than trying to fix the specific issue. Other advantages: People are able to concentrate on the specific changes during the review and the comment history stays focused. -- To view, visit https://review.coreboot.org/c/flashrom/+/67664 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If28fbe09ad685082152aa3a7e8d5a150169aee9e Gerrit-Change-Number: 67664 Gerrit-PatchSet: 6 Gerrit-Owner: Alexander Goncharov <chat@joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm@chromium.org> Gerrit-Reviewer: Thomas Heijligen <src@posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Felix Singer <felixsinger@posteo.net> Gerrit-Attention: Thomas Heijligen <src@posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm@chromium.org> Gerrit-Attention: Alexander Goncharov <chat@joursoir.net> Gerrit-Comment-Date: Sat, 01 Oct 2022 23:31:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger@posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm@chromium.org> Gerrit-MessageType: comment
participants (1)
-
Felix Singer (Code Review)