Patch bomb from Unity Technologies
Mads Kiilerich
mads at kiilerich.com
Fri Jul 18 13:38:43 EDT 2014
On 07/18/2014 02:02 AM, Sean Farley wrote:
> I have finally gotten around to reviewing this pull request. These
> changes are great and should be merged!
Thanks a lot for taking the time to do that. We at Unity want to invest
in this, not own it ;-)
> My only comments are small nits about styling the commit message to have
> the first line less than 80 characters but that shouldn't block this
> merge. Instead, I would suggest bikeshedding a code-style guide for
> would-be contributors.
>
> A few notes I made while reviewing:
>
> - most style choices (indentation, css, if-blocks in html) are inherited
> so others should feel free to send patches later to clean that up
Yes, for now my general guideline is somewhere between pep8 and
Mercurial and consistency.
> - I like 38ec32b1ef50 and will use that for my evolution changesets
Yes, I saw your example on https://smf.io/ktest/changelog - very nice!
> - The biggest change is probably 4666729c8bb8 since it allows pull
> requests to be updated to a new one. I think this is great and should
> use mercurial evolution in the future :-)
Yes, somehow.
The thing I added could be considered scripting of the most obvious
workaround - it doesn't change the data model at all.
Long term, I would like to support different models. I really don't like
that changesets automagically are added to an existing PR ... but I
guess some people are used to that and want that. I guess that depends
on how much people deal with history. Let's see ;-)
> I like the general direction and will also use what I learned to improve
> my own patches.
Thanks. I tweaked the queue a bit and pushed it. Some new minor changes
and bugfixes was added and some patches has been postponed a bit.
/Mads
More information about the kallithea-general
mailing list