[flashrom] please reject my two previous letters

Antonio Ospite ao2 at ao2.it
Sat Jan 10 22:14:01 CET 2015


On Sat, 10 Jan 2015 13:31:08 +0300
Boris Baykov <dev at 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.

> 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
>

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 at 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 Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?




More information about the flashrom mailing list