On Mon, Jun 19, 2017 at 05:13:18PM +0200, m...@datanom.net wrote: > From: Michael Rasmussen <m...@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 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
