[pve-devel] [PATCH v6 pve-storage 0/6] FreeNAS storage plugin

[Fabian Grünbichler]

On Mon, Jun 19, 2017 at 05:13:18PM +0200, mir at datanom.net wrote:
> From: Michael Rasmussen <mir at datanom.net>
> 
> (Resending, Diregard previous)

this is almost the opposite of what I requested (I haven't looked at the
actual changes, I am talking about the organization of the patches).
I'll do further review once you split up patch #1 into logical chunks
like (note that these are just examples!):

1. add skeleton FreeNASPlugin.pm with properties, options, type
2. add basic FreeNAS API interaction code
3. add basic iscsiadm interaction code
4. implement FreeNAS storage operations (note: this could very well be
more than one commit)
5. implement high-level PVE Storage Plugin API (note: this as well)
6. improve FreeNAS API access by adding pagination support (just to give
you an example of what could be split out into a separate "feature"
commit, feel free to include this in patch #2)
...
N. enable plugin by registering in Storage.pm and referencing in the
build scripts

please integrate patches 2-6 (as well as further fixes) into these
commits, unless they really deserve to get their own commits. don't put
unrelated changes into a single commit.

a simple way to get from the current state to something like this is,
assuming you are currently in a clean checkout of your v6:

"git checkout -b freenaspluginv7" (to check out a new v7 branch based on
v6)
"git reset HEAD~6" (to throw away the last 6 commits but keep the
changed files)
"git add -N PVE/Storage/FreeNASPlugin.pm" (to add an empty
FreeNASPlugin.pm, otherwise "add -p" does not work)

followed by the following for each commit:
"git add -p PVE/Storage/FreeNASPlugin.pm" (to selectively add parts of
the plugin code to the staging area)
"git diff --cached" (to review what you are about to commit)
"git commit" (to create a commit)

note that you can rework your commits (e.g., change the commit message,
order, or add additional fixes) using "git rebase -i"[1] as long as your
working directory is clean.

once you are happy with your commits:
"git format-patch -v7 --signoff --cover-letter HEAD~N" (where N is the
number of commits)

note that "add -p" is interactive, so for the first few big splits you
probably need to press "e" to edit manually, and delete all lines which
you don't want to add (deleting them just means not adding them to the
staging area, they are not completely thrown away). since this is new
code, it is okay to be a bit sloppy with the "use" statements and so on,
since you can postpone the actual inclusion to the last commit in the
series. but please try to stay within reasonable boundaries (e.g.,
adding "activate_volume" in the first commit without any of the
underlying helper methods is probably not okay ;))

if something about this is unclear, just ask!

1: short introduction to git rebase -i :
https://robots.thoughtbot.com/git-interactive-rebase-squash-amend-rewriting-history