[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: The review process for new "complicated" stuff (Re: DriverDisc v3 integration)

Hash: SHA1

On Wed, 2 Dec 2009, Martin Sivak wrote:

Hi David,

I didn't know the git add -i feature and so I didn't see any way how to handle this "nicely". I agree that review process is needed so I did submited the patches by-file at least and tried to explain what is going on. Jeremy's comment was just too aggresive for my taste. So I might have responded without enough thinking about the answer.

No, it's ok.  I definitely think people should speak up if they have concerns
with team processes.  In the past couple of years, we have changed a lot of
things.  New development tools as well as new processes.

This is true, but if those mistakes are the responsibility of another
component, we don't have to pull out our hair.  While cpio is a simple
there's no reason we should get in to the business of owning cpio

The wrapper will be still ours to fix.. I was merely comparing the complextity of wrapper to the cpio code itself. Also my memory concerns still apply. Also, we should be careful in making stage1 bigger by adding another libraries.

Sure, the wrapper will still be ours, but it won't be cpio code.  My concern
is adding in cpio code when there is already perfectly usable cpio code
elsewhere.  Increasing the size of stage1 is a concern, sure, but libarchive
on my netbook is 230k and I'm pretty sure we have all libraries it links well
in the initrd.img already.  Alternatively, /bin/cpio appears to be 128k.
Either way, I'm in favor of taking that route rather than owning cpio code
directly in the loader.

Personally, I've been using stacked git (stgit) to manage patches.
What I
like about it is I can manage revisions to patches during review more
I have my own local workflow for noting when patches are reviewed or
not (I
move them to another local branch).  Whatever is left in the original
is still out for review.

For me, the emails on the list contain useful information but are
once I process them.

So you sync all the patches to your git repo? Do you use git am for that? stgit is nice idea though, I might try that.

I use a lot of local branches.  Here's a basic breakdown of my workflow:

- - In my local anaconda repo, I create new branches off of the release branch
  to work on a bug fix or new feature.  For example, if I'm fixing a
  particular RHEL 5 bug, I might create 'rhel5-bzXXXXXX' off of my local
  rhel5-branch.  If I'm working on a new feature for Fedora, I might do
  'master-feature' off of master.

- - I work on each thing independently.  I continually fetch from the central
  git repo and rebase the origin branches.  When it comes time for me to
  submit patches to the list, I create a new branch called 'master-staging'
  or maybe 'rhel5-staging' off of the appropriate release branch.

- - In the staging branches, I use git am -or- git merge to pull in all of
  the individual work branches I want to send to the list.

- - The staging branch is where I construct a header email and use git
  format-patch to send patches to the list for review.

- - I use stgit on the staging branch to manage revisions to those patches.
  Sometimes I just create a new commit and then use 'git rebase -i' to
  squash the revision in to the original patch.  I find stgit more useful
  for larger patch sets where a lot of comments are coming in.

- - When reviews are over, I use 'git am' to pick the patches from the staging
  branch on to the main branch.  Whatever patches never received review, I
  leave them in the staging branch.  This is how I track what people have
  reviewed or not.  I never merge the staging branch on to the main release
  branch because I don't want us to have the merge commits in our history.

That may seem like a lot, but it's sort of how I've gotten used to using git.
Branches are cheap and I make use of them.  It's a nice way to divide up work.

For the illustrated version:

master --------<-------- "git am" -------<------+
  |                                             |
  +--master-feature ---\                        |
  |                     +-- master-staging -->--+
  +--master-bugfix ----/

rhel5-branch -----<----- "git am" ------<-------+
  |                                             |
  +--rhel5-bz123456 ---\                        ^
  |                     \                       |
  +--rhel5-s390junk -----+-- rhel5-staging -->--+
  |                     /
  +--rhel5-urgent -----/

- -- David Cantrell <dcantrell redhat com>
Red Hat / Honolulu, HI

Version: GnuPG v1.4.10 (GNU/Linux)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]