| Message ID | 20200929142501.1057-1-ani@anisinha.ca (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5] Add a comment in bios-tables-test.c to clarify the reason behind approach | expand |
Request to queue this patch with the next pull. > On Sep 29, 2020, at 7:55 PM, 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: > v5: incorporated suggestion from eric. > v4: more updates as per Igor's suggestion. Dropped some comment lines. added > ack'd by line. > v3: review feedback incorporared and actual comment in the code reworded. > v2: cosmetic - commit log reworded. > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c > index 3c09b844f9..fc7aaaf82c 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 the maintainer 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 > + * 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 > -- > 2.17.1 >
On 12/10/2020 16.07, Ani Sinha wrote:
> Request to queue this patch with the next pull.
I can take it via qtest-next in case Michael / Igor do not plan to send a
pull request any time soon.
Thomas
On Mon, Oct 12, 2020 at 19:43 Thomas Huth <thuth@redhat.com> wrote: > On 12/10/2020 16.07, Ani Sinha wrote: > > Request to queue this patch with the next pull. > > I can take it via qtest-next in case Michael / Igor do not plan to send a > pull request any time soon. Michael or Igor, care to comment? > > Thomas > >
On Thu, Oct 15, 2020 at 04:58:37PM +0530, Ani Sinha wrote: > > > On Mon, Oct 12, 2020 at 19:43 Thomas Huth <thuth@redhat.com> wrote: > > On 12/10/2020 16.07, Ani Sinha wrote: > > Request to queue this patch with the next pull. > > I can take it via qtest-next in case Michael / Igor do not plan to send a > pull request any time soon. > > > Michael or Igor, care to comment? Go ahead Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > > > > Â Thomas > >
On 15/10/2020 13.28, Ani Sinha wrote: > > > On Mon, Oct 12, 2020 at 19:43 Thomas Huth <thuth@redhat.com > <mailto:thuth@redhat.com>> wrote: > > On 12/10/2020 16.07, Ani Sinha wrote: > > Request to queue this patch with the next pull. > > I can take it via qtest-next in case Michael / Igor do not plan to send a > pull request any time soon. > > > Michael or Igor, care to comment? FYI, patch got merged: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=07f5903c852de10fa7 Thomas
On Thu, Oct 15, 2020 at 7:29 PM Thomas Huth <thuth@redhat.com> wrote: > > On 15/10/2020 13.28, Ani Sinha wrote: > > > > > > On Mon, Oct 12, 2020 at 19:43 Thomas Huth <thuth@redhat.com > > <mailto:thuth@redhat.com>> wrote: > > > > On 12/10/2020 16.07, Ani Sinha wrote: > > > Request to queue this patch with the next pull. > > > > I can take it via qtest-next in case Michael / Igor do not plan to send a > > pull request any time soon. > > > > > > Michael or Igor, care to comment? > > FYI, patch got merged: > > https://git.qemu.org/?p=qemu.git;a=commitdiff;h=07f5903c852de10fa7 Thanks!
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index 3c09b844f9..fc7aaaf82c 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 the maintainer 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 + * 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