| Message ID | 20200924090925.18915-1-ani@anisinha.ca |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] Add a comment in bios-tables-test.c to clarify the reason behind approach | expand |
Michael, Please queue this one for the next pull as well. On Sep 24, 2020, 14:39 +0530, Ani Sinha <ani@anisinha.ca>, wrote: > A comment is added in bios-tables-test.c that explains the reasoning > behind the process of updating the ACPI table blobs when new tests are added > or old tests are modified or code is committed that affect tests. The > explanation would help future contributors follow the correct process when > making code changes that affect ACPI tables. > > Signed-off-by: Ani Sinha <ani@anisinha.ca> > Acked-by: Igor Mammedov <imammedo@redhat.com> > --- > tests/qtest/bios-tables-test.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > Changelog: > v2: cosmetic - commit log reworded. > v3: review feedback incorporared and actual comment in the code reworded. > v4: more updates as per Igor's suggestion. Dropped some comment lines. added > ack'd by line. > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c > index b514b70b62..34e2e1c55b 100644 > --- a/tests/qtest/bios-tables-test.c > +++ b/tests/qtest/bios-tables-test.c > @@ -11,7 +11,7 @@ > */ > > /* > - * How to add or update the tests: > + * How to add or update the tests or commit changes that affect ACPI tables: > * Contributor: > * 1. add empty files for new tables, if any, under tests/data/acpi > * 2. list any changed files in tests/qtest/bios-tables-test-allowed-diff.h > @@ -38,6 +38,11 @@ > * $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh > * 6. Now commit any changes to the expected binary, include diff from step 4 > * in commit log. > + * Expected binary updates needs to be a separate patch from the code that > + * introduces changes to ACPI tables. It lets maintainer to drop > + * and regenerate binary updates in case of merge conflicts. Further, a code > + * change is easily reviewable but a binary blob is not (without doing a > + * diassemly). > * 7. Before sending patches to the list (Contributor) > * or before doing a pull request (Maintainer), make sure > * tests/qtest/bios-tables-test-allowed-diff.h is empty - this will ensure > -- > 2.17.1 > <html xmlns="http://www.w3.org/1999/xhtml"> <head> <title></title> </head> <body> <div name="messageBodySection"> <div dir="auto">Michael,<br /> <br /> Please queue this one for the next pull as well.</div> </div> <div name="messageReplySection">On Sep 24, 2020, 14:39 +0530, Ani Sinha <ani@anisinha.ca>, wrote:<br /> <blockquote type="cite" style="border-left-color: grey; border-left-width: thin; border-left-style: solid; margin: 5px 5px;padding-left: 10px;">A comment is added in bios-tables-test.c that explains the reasoning<br /> behind the process of updating the ACPI table blobs when new tests are added<br /> or old tests are modified or code is committed that affect tests. The<br /> explanation would help future contributors follow the correct process when<br /> making code changes that affect ACPI tables.<br /> <br /> Signed-off-by: Ani Sinha <ani@anisinha.ca><br /> Acked-by: Igor Mammedov <imammedo@redhat.com><br /> ---<br /> tests/qtest/bios-tables-test.c | 7 ++++++-<br /> 1 file changed, 6 insertions(+), 1 deletion(-)<br /> <br /> Changelog:<br /> v2: cosmetic - commit log reworded.<br /> v3: review feedback incorporared and actual comment in the code reworded.<br /> v4: more updates as per Igor's suggestion. Dropped some comment lines. added<br /> ack'd by line.<br /> <br /> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c<br /> index b514b70b62..34e2e1c55b 100644<br /> --- a/tests/qtest/bios-tables-test.c<br /> +++ b/tests/qtest/bios-tables-test.c<br /> @@ -11,7 +11,7 @@<br /> */<br /> <br /> /*<br /> - * How to add or update the tests:<br /> + * How to add or update the tests or commit changes that affect ACPI tables:<br /> * Contributor:<br /> * 1. add empty files for new tables, if any, under tests/data/acpi<br /> * 2. list any changed files in tests/qtest/bios-tables-test-allowed-diff.h<br /> @@ -38,6 +38,11 @@<br /> * $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh<br /> * 6. Now commit any changes to the expected binary, include diff from step 4<br /> * in commit log.<br /> + * Expected binary updates needs to be a separate patch from the code that<br /> + * introduces changes to ACPI tables. It lets maintainer to drop<br /> + * and regenerate binary updates in case of merge conflicts. Further, a code<br /> + * change is easily reviewable but a binary blob is not (without doing a<br /> + * diassemly).<br /> * 7. Before sending patches to the list (Contributor)<br /> * or before doing a pull request (Maintainer), make sure<br /> * tests/qtest/bios-tables-test-allowed-diff.h is empty - this will ensure<br /> --<br /> 2.17.1<br /> <br /></blockquote> </div> </body> </html>
On 9/24/20 4:09 AM, Ani Sinha wrote: > A comment is added in bios-tables-test.c that explains the reasoning > behind the process of updating the ACPI table blobs when new tests are added > or old tests are modified or code is committed that affect tests. The > explanation would help future contributors follow the correct process when > making code changes that affect ACPI tables. > > Signed-off-by: Ani Sinha <ani@anisinha.ca> > Acked-by: Igor Mammedov <imammedo@redhat.com> > --- > tests/qtest/bios-tables-test.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > * 6. Now commit any changes to the expected binary, include diff from step 4 > * in commit log. > + * Expected binary updates needs to be a separate patch from the code that > + * introduces changes to ACPI tables. It lets maintainer to drop s/maintainer to/the maintainer/ > + * and regenerate binary updates in case of merge conflicts. Further, a code > + * change is easily reviewable but a binary blob is not (without doing a > + * diassemly). disassembly > * 7. Before sending patches to the list (Contributor) > * or before doing a pull request (Maintainer), make sure > * tests/qtest/bios-tables-test-allowed-diff.h is empty - this will ensure > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index b514b70b62..34e2e1c55b 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -11,7 +11,7 @@ */ /* - * How to add or update the tests: + * How to add or update the tests or commit changes that affect ACPI tables: * Contributor: * 1. add empty files for new tables, if any, under tests/data/acpi * 2. list any changed files in tests/qtest/bios-tables-test-allowed-diff.h @@ -38,6 +38,11 @@ * $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh * 6. Now commit any changes to the expected binary, include diff from step 4 * in commit log. + * Expected binary updates needs to be a separate patch from the code that + * introduces changes to ACPI tables. It lets maintainer to drop + * and regenerate binary updates in case of merge conflicts. Further, a code + * change is easily reviewable but a binary blob is not (without doing a + * diassemly). * 7. Before sending patches to the list (Contributor) * or before doing a pull request (Maintainer), make sure * tests/qtest/bios-tables-test-allowed-diff.h is empty - this will ensure