Re: [Mingw-w64-public] [PATCH 1/5] headers: add dxvahd.idl
On 10/03/2020 12:48, Steve Lhomme wrote: On 2020-03-10 11:31, Jacek Caban wrote: On 10/03/2020 07:34, Steve Lhomme wrote: You mean squash all 5 patches into one or just the first two ? In general I prefer to split commits so if something breaks it's easier to know what is wrong in a particular commit and possibly revert it, without losing all changes. Splitting patches is generally a good habit, but only when it makes sense. In this case, you know that the interface that you're adding in patch 1 is broken (and you already have a fix for that), so how can we review patch 1? Patch one is generating the "same" file from an IDL file. With the same incorrect order and with the same tricks like the CALLBACK one or the interface warning during compilation. Modifying an IDL file is easier and has less duplicate parts so it's easier to get there sooner in the patchset. That avoids modifying more lines later and making more mistake. Reviewing this patch means verifying it produces more or less the same file without breaking anything. I looked only at .idl part and for some reason operated under assumption that we don't have that header yet. Sorry about that, split version of the patch is actually better in that case. I will review it later today. Thanks, Jacek ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [PATCH 1/5] headers: add dxvahd.idl
On 2020-03-10 11:31, Jacek Caban wrote: On 10/03/2020 07:34, Steve Lhomme wrote: You mean squash all 5 patches into one or just the first two ? In general I prefer to split commits so if something breaks it's easier to know what is wrong in a particular commit and possibly revert it, without losing all changes. Splitting patches is generally a good habit, but only when it makes sense. In this case, you know that the interface that you're adding in patch 1 is broken (and you already have a fix for that), so how can we review patch 1? Patch one is generating the "same" file from an IDL file. With the same incorrect order and with the same tricks like the CALLBACK one or the interface warning during compilation. Modifying an IDL file is easier and has less duplicate parts so it's easier to get there sooner in the patchset. That avoids modifying more lines later and making more mistake. Reviewing this patch means verifying it produces more or less the same file without breaking anything. Please merge them all. OK. I'll do that. ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [PATCH 1/5] headers: add dxvahd.idl
On 10/03/2020 07:34, Steve Lhomme wrote: You mean squash all 5 patches into one or just the first two ? In general I prefer to split commits so if something breaks it's easier to know what is wrong in a particular commit and possibly revert it, without losing all changes. Splitting patches is generally a good habit, but only when it makes sense. In this case, you know that the interface that you're adding in patch 1 is broken (and you already have a fix for that), so how can we review patch 1? Please merge them all. Thanks, Jacek ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [PATCH 1/5] headers: add dxvahd.idl
You mean squash all 5 patches into one or just the first two ? In general I prefer to split commits so if something breaks it's easier to know what is wrong in a particular commit and possibly revert it, without losing all changes. On 2020-03-09 16:45, Jacek Caban wrote: Hi Steve, On 04.03.2020 15:27, Steve Lhomme wrote: Use the generated dxvahd.h --- mingw-w64-headers/Makefile.am | 1 + mingw-w64-headers/include/dxvahd.h | 841 ++- mingw-w64-headers/include/dxvahd.idl | 427 ++ 3 files changed, 990 insertions(+), 279 deletions(-) create mode 100644 mingw-w64-headers/include/dxvahd.idl Please merge those patches. There is no need to add methods in wrong order only to reorder them later in the series. Thanks, Jacek ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [PATCH 1/5] headers: add dxvahd.idl
Hi Steve, On 04.03.2020 15:27, Steve Lhomme wrote: Use the generated dxvahd.h --- mingw-w64-headers/Makefile.am| 1 + mingw-w64-headers/include/dxvahd.h | 841 ++- mingw-w64-headers/include/dxvahd.idl | 427 ++ 3 files changed, 990 insertions(+), 279 deletions(-) create mode 100644 mingw-w64-headers/include/dxvahd.idl Please merge those patches. There is no need to add methods in wrong order only to reorder them later in the series. Thanks, Jacek ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
