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