On Sat, 10 Jan 2015 01:34:34 +0300 Boris Baykov dev@borisbaykov.com wrote:
On Fri, 9 Jan 2015 22:49:56 +0100 Stefan Tauner stefan.tauner@alumni.tuwien.ac.at
On Fri, 9 Jan 2015 23:54:28 +0300 Boris Baykov dev@borisbaykov.com wrote:
[...]
Please explain how is better to split this patch to make it easy for review.
We want to verify all changes to flashrom in quite some detail to keep it as stable as it is. A single huge patch like yours takes a very long time to fully understand. It is probably not even possible to review such a big change in one go without making some mistakes or overlook error of the original author. Smaller independent patches could be reviewed entirely without getting too much of a headache. Does that make sense to you?
[...]
Unfortunately I can't imagine how to split this patch in parts because all the patch logically has one idea and that's why my changes in different files are connected to other changes and they work together.
As you said, splitting by files in general does not make sense, but splitting by "feature" does.
The idea is that an incremental patchset is a coarse (and cleaned up) version of your development history.
Thanks, Antonio for the excellent explanation! It looks like a set of nesting dolls from traditional russian culture where each next doll goes inside of previous one :-)
Basing on your list I developed the following order of 10 + 2 patches, where basically 10 patches and added extra 2 small patches 3a and 4a for explanation purposes only to provide an example how to use new features. This means that to make 4BA version we should patch 1+2+3+4+5+6+7+8+9+10, and 3a and 4a can be after 3 and 4 to show code examples. Is it ok?
Please explain why do you think to split the first patch into two parts ?
- base support for the JEDEC extensions can go into a separate patch;
- flashrom hooking for the extensions and for 4 byte addressing can go into a dedicated patch;
From one side the idea is good from another side we'll receive the patch (first one) which doesn't make any sense besides of source size increase.
1. 4BA: Basic support for 4-bytes addressing mode 2. 4BA: Winbond W25Q256.V chip declaration 3. 4BA: Support of Extended Address Register 3a.4BA: Temporary W25Q256.V declaration update to work using Ext.Addr.Reg 4. 4BA: Support for new direct 4BA instructions + W25Q256.V declaration update 4a.4BA: Temporary W25Q256.V declaration update to work via direct funcs 5. 4BA: Progress visualization 6. 4BA: SFDP 1.6 parser for compliance with JESD216B 7. 4BA: SFDP 1.6 simulation (for testing purposes) 8. 4BA: SFDP downgrade to 1.5 to support Micron chips 9. 4BA: Some small visual changes for 4-byte addresses 10.4BA: Temporary banner fix about the 4BA patch
So, I can split the patch today and resubmit the patches set here.
We have now build 1866 already. Do I need to update my patch for this build?