On Sat, 10 Jan 2015 13:31:08 +0300 Boris Baykov dev@borisbaykov.com wrote:
[...]
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?
Great, more than 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.
It was just an example, I had only skimmed through the code not actually read it. You know the code better, you should decide what the best way to split the patch is.
- 4BA: Basic support for 4-bytes addressing mode
- 4BA: Winbond W25Q256.V chip declaration
- 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
Sounds pretty good to me, and in the future using a similar approach from the start will make things easier for you too.
In the final submission the numeration will be just incremental (like in a list): 3a becomes 4 and so on; but in principle a development history looks more like a graph indeed, just like you pictured it.
One thing to remember is that, as a general rule, the code should still "work" after each patch, at least it should still compile.
So, I can split the patch today and resubmit the patches set here.
If you feel like it, and you have some time, try using "git": play with local branches and with the "git rebase" command, you can clone the flashrom svn repository as a git repository using the "git svn" command.
Since your patch was based on top of r1861, a possible way to operate can be this:
# Clone the svn repository (this will get the _latest_ commit) git svn clone svn://flashrom.org/flashrom/trunk flashrom cd flashrom
# Start a local temporary branch named "temp-add-4BA-support", # starting from r1861 git checkout -b temp-add-4BA-support $(git svn find-rev r1861)
# Apply your patch to this branch, and commit it git apply flashrom-0.9.7-r1861_4ba.patch git commit -a -m "Huge patch"
# Rebase the branch on top of the latest revision git rebase master
# Export the rebased patch, the file will be named 0001-Huge-patch.patch git format-patch -1
# Switch to the master branch and delete the temp branch git checkout master git branch -D temp-add-4BA-support
# Start the actual branch from the latest revision, that will be # the one you'll be working on git checkout -b add-4BA-support
# Apply the patch (you will see warning about trailing spaces, you can # fix those at the end) and check the diff git apply 0001-Huge-patch.patch git diff
# Use "git add -p" to select the pieces of the huge patch targeted for # the _first_ dedicated patch, when finished with the first patch, commit. git add -p git commit
# "git commit" will present you an editor to write your commit message, # for the first patch it could be something like this
# (ignore the -----... lines):
Basic support for 4-bytes addressing mode
Signed-off-by: Boris Baykov dev@borisbaykov.com
Repeat the "git add -p" thing for the other patches, until "git add -p" does not do anything anymore.
After that you can see the split patchset by comparing the master branch with the add-4BA-support branch: git whatchanged --reverse master..
Note that "git add -p" gets some time to get used to, that may discourage you, but again I am just proposing a _possible_ way to operate, you should use what is more comfortable for you.
If you don't want to learn git now, just ignore what I wrote, it may even be more confusing to newcomers, I know. :)
Ciao, Antonio
Antonio, I appreciate your help with git. It's very useful. I thought about making new branch for this patch. Right now I have no time for this and I've already split this patch into 10+2 parts (+2 just examples, don't apply) and diffed it by Linux diff(1) instead of SVN or Git. It's enough for review. Later I probably add the branch and prepare scripts to do git diff.
My big thanks to you for ideas and explanation! It seems that the patch will be available for review soon. I've sent all parts here already.
Besides of 10+2 parts I've done, packed and sent the full SVN diff of last version of the patch because I fixed some small points while splitting. So, to apply the patch now we can use this.
Also I should tell that all parts of the patch is sent with spaces instead of tabs. To go over this trouble I packed and attacked each part to its letter. Use .patch files from attached .bzips.