[Mesa-dev] [PATCH] gallium/swr: update rasterizer (532172)

Jordan Justen jordan.l.justen at intel.com
Wed Mar 23 05:52:27 UTC 2016


On 2016-03-22 20:55:10, Rowley, Timothy O wrote:
> 
> > On Mar 22, 2016, at 3:51 PM, Justen, Jordan L <jordan.l.justen at intel.com> wrote:
> > 
> > What does 532172 in the subject refer to?
> 
> swr rasterizer development happens in another source control system.
> 532172 is a revision id to checkpoint where we’ve pushed the changes
> publicly.
> 
> > From this commit message, it seems clear that this single patch is
> > doing a whole lot. Usually that's a good sign that it should be split
> > into multiple patches.
> > 
> > However, since this is only changing your driver, you can probably
> > take any sort of patches that you like. :)
> > 
> > There is arguably little value to sending out a patch like this, since
> > it is very difficult to review. In other words, perhaps if you are
> > going to make big, unreviewable patches like this that only change
> > your driver, then you might as well just push them straight away.
> > 
> > (But, it would be better, in my opinion, to try to split up the
> > changes and let them be reviewed.)
> 
> Yes, there’s a lot in this patch. I froze the public version of the
> rasterizer when I began the upstreaming process mid February, so
> this is syncing up with about a month’s worth of development.
> 
> I also have this change as a series of 81 commits. Not sure if that
> would be preferable by the community or if people would be
> interested in reviewing the series, as issues with early commits
> might already be addressed later in the patch set.
> 

There seems to be some things working against community code review.

* Expected broken commits earlier in the series (We would normally ask
  that commits are cleaned up before posting them.)

* External development (What would happen to any code review asking
  for reworks, given that the patches are already merged elsewhere?)

* A large backlog of changes. :)

For those reasons, I don't see much value in posting this, or the 81
patches to mesa-dev. Maybe going fwd, there won't be such a backlog,
and code review would then be possible. (And, of course anything
outside the openswr driver code would require code review.)

I still think it would be better to see the 81 commits split up in the
history as long as they won't cause problems for others. Since most
people are unlikely to be building openswr, I don't think the commits
will affect them.

We rarely use merges, but perhaps it is appropriate since openswr is
developed externally. You could start a branch at the last openswr
commit, add your 81 commits. Then you could merge the resulting branch
into master.

-Jordan


More information about the mesa-dev mailing list