> On Sat, 10 Jan 2015 01:34:34 +0300 > Boris Baykov <[email protected]> wrote:
>> On Fri, 9 Jan 2015 22:49:56 +0100 >> Stefan Tauner <[email protected]> >> >> > On Fri, 9 Jan 2015 23:54:28 +0300 >> > Boris Baykov <[email protected]> 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? _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
