| Re: [cross-project-issues-dev] 1000 line limit for contributions |
FWIW, I agree with Ed and Christian. My standing guidance to contributors to Eclipse ICE is to keep their contributions below the limit so that I can review them personally. If they go above the limit by just a little then I ask them to split it, but if they break it by 100 lines or so I just submit it for IP review.
I've never had a review of this type take longer than two weeks after I submitted it (I think). As previously suggested, I imagine that ECE just got it the way.
Jay
I totally agree on this and I definitly think that blank lines shouldnt
count. :-)
And since quit a lot of the IP personal was at ECE that probably didnt
speed up this case.
I did a quick search on ipzilla and did find this….
https://dev.eclipse.org/ipzilla/buglist.cgi?query_format=advanced&short_des
c_type=allwordssubstr&short_desc=&long_desc_type=substring&long_desc=Gerrit
+code+contribution&bug_file_loc_type=allwordssubstr&bug_file_loc=&keywords_
type=allwords&keywords=&emailassigned_to1=1&emailtype1=substring&email1=&em
ailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&
bugidtype=include&bug_id=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=
doit&order=Reuse+same+sort+as+last+time&field0-0-0=noop&type0-0-0=noop&valu
e0-0-0=
Not many cases are resolved and most are very very new so maybe the review
triggered through Gerrit is a new thing that needs some time for the IP
Team to get used to…. (I searched for „Gerrit code contribution“ in the
comment“)
christian
Am 19.11.15, 11:47 schrieb "cross-project-issues-dev-bounces@xxxxxxxxxxx
on behalf of Ed Merks" unter <cross-project-issues-dev-bounces@xxxxxxxxxxx
on behalf of ed.merks@xxxxxxxxx>:
>Yes, I believe it's an important aspect of Eclipse that makes it stand
>out as the best place to be if you want the broadest possible community
>of adopters. Of course this benefit doesn't come without a cost and of
>course that can be frustrating. In a specific case of a contribution
>that consists of a relatively smaller changes to the framework/tool with
>a relatively larger addition of test case(s), it would seem reasonable
>to split the two, if it's important that the change to the
>tools/framework show up as quickly as possible.
>
>I certainly don't suggest gaming the system, though I do tend to point
>out to the IP committee all the ways it can be gamed, and will be gamed
>by developers who are frustrated and don't take the issue seriously. I
>ask questions such as how long can a line be? One can fit quite a lot
>on a line line and reformat it later. Also, why should a blank line
>count for anything? Is a line with just a curly brace on it really
>IP? And yes, of course I make them aware that contributions can be
>split into smaller chunks...
>
>Perhaps this specific review period overlapped with EclispeCon Europe
>where we had the pleasure of spending personal time with the with the IP
>staff...
>
>
>On 19/11/2015 11:31 AM, Christian Campo wrote:
>> Wouldnt it be worth to hear what the IP Team has to say why this took so
>> long ? I see that Sharon appologized on the CQ that it took so long.
>>That
>> made me believe that this was an exception.
>>
>> Does every CQ with 1000 lines take so long ? What is the experience of
>> others about reviews with code contributions.
>> As I remember vaguely (and that might be incorrect) the IP team runs
>> automatic scans over the code, but I am not sure what else they do.
>>
>> I for once believe the work of the IP Team is important and one of the
>> core values of the EF vs say Github and I take it serious.
>>
>> Just my 2 cents
>>
>> christian
>>
>> Am 19.11.15, 11:22 schrieb "cross-project-issues-dev-bounces@xxxxxxxxxxx
>> on behalf of Sievers, Jan" unter
>> <cross-project-issues-dev-bounces@xxxxxxxxxxx on behalf of
>> jan.sievers@xxxxxxx>:
>>
>>> If everybody tells me there are ways to dodge around that rule (and of
>>> course I know there are), the question arises why do we have the rule
>>>in
>>> the first place. Seems a little absurd to me.
>>>
>>> the effort is not minimal if I have to artificially split up commits.
>>> Or maybe you expect me to explain to contributors:
>>>
>>> "look, we have this process but nobody takes it serious anyway. so
>>>please
>>> split up your commit into several < 1000 LOC chunks" ?
>>>
>>> Best Regards,
>>> Jan
>>>
>>>
>>>
>>> On 19/11/15 11:00, "cross-project-issues-dev-bounces@xxxxxxxxxxx on
>>> behalf of Ed Willink" <cross-project-issues-dev-bounces@xxxxxxxxxxx on
>>> behalf of ed@xxxxxxxxxxxxx> wrote:
>>>
>>>> Hi
>>>>
>>>> Presumably you put tests in a separate plugin, so splitting off the
>>>> tests as a separate contribution gets you twice the limit with minimal
>>>> effort.
>>>>
>>>> Perhaps a 10000 line limit might be appropriate for non-deliverable
>>>>code
>>>> such as tests and build tools.
>>>>
>>>> Regards
>>>>
>>>> Ed Willink
>>>>
>>>>
>>>>
>>>> On 19/11/2015 09:49, Sievers, Jan wrote:
>>>>> Hi,
>>>>>
>>>>> in the course of
>>>>>
>>>>> https://bugs.eclipse.org/bugs/show_bug.cgi?id=477328
>>>>>
>>>>>
>>>>> we had a contribution that slightly exceeded 1000 lines and thus
>>>>> needed a CQ.
>>>>> It took about one month to review it.
>>>>>
>>>>> I am sure the legal team does its very best to keep up with the load,
>>>>> so the following is in no way a criticism of the
>>>>> people who actually do the legal review.
>>>>>
>>>>> Rather take it as food for thought to whoever set up this rule.
>>>>>
>>>>> IMHO the 1000 line rule is effectively setting the wrong incentives
>>>>> for a thriving opensource project.
>>>>>
>>>>> Here is why I think so:
>>>>>
>>>>>
>>>>> The most diligent contributors add a lot of tests to their patch to
>>>>> prove it works.
>>>>> This is a good thing and we actively encourage contributors to
>>>>> thoroughly test.
>>>>> Test code can easily outweigh productive code being tested in terms
>>>>>of
>>>>> LOC.
>>>>> However this means the most diligent contributors, i.e. the ones you
>>>>> want to attract, are more likely to hit the 1000 line limit.
>>>>> Instead of thanking them for their hard work, we effectively punish
>>>>> them with an extra month or more wait time before their patch can be
>>>>> merged.
>>>>> Apart from that, the 1000 line limit seems arbitrary to me because
>>>>> technically you can split up any commit into any number
>>>>> of smaller commits below the 1000 line limit.
>>>>>
>>>>> Best Regards,
>>>>> Jan
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cross-project-issues-dev mailing list
>>>>> cross-project-issues-dev@xxxxxxxxxxx
>>>>> To change your delivery options, retrieve your password, or
>>>>> unsubscribe from this list, visit
>>>>> https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
>>>>>
>>>> _______________________________________________
>>>> cross-project-issues-dev mailing list
>>>> cross-project-issues-dev@xxxxxxxxxxx
>>>> To change your delivery options, retrieve your password, or
>>>>unsubscribe
>>> >from this list, visit
>>>> https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
>>> _______________________________________________
>>> cross-project-issues-dev mailing list
>>> cross-project-issues-dev@xxxxxxxxxxx
>>> To change your delivery options, retrieve your password, or unsubscribe
>> >from this list, visit
>>> https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
>> -------------------------------------------------------------
>> compeople AG
>> Untermainanlage 8
>> 60329 Frankfurt/Main
>> fon: +49 (0) 69 / 27 22 18 0
>> fax: +49 (0) 69 / 27 22 18 22
>> web: www.compeople.de
>>
>> Vorstand: Jürgen Wiesmaier
>> Aufsichtsratsvorsitzender: Christian Glanz
>>
>> Sitz der Gesellschaft: Frankfurt/Main
>> Handelsregister Frankfurt HRB 56759
>> USt-IdNr. DE207665352
>> -------------------------------------------------------------
>> _______________________________________________
>> cross-project-issues-dev mailing list
>> cross-project-issues-dev@xxxxxxxxxxx
>> To change your delivery options, retrieve your password, or unsubscribe
>>from this list, visit
>> https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
>
>_______________________________________________
>cross-project-issues-dev mailing list
>cross-project-issues-dev@xxxxxxxxxxx
>To change your delivery options, retrieve your password, or unsubscribe
>from this list, visit
>https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
-------------------------------------------------------------
compeople AG
Untermainanlage 8
60329 Frankfurt/Main
fon: +49 (0) 69 / 27 22 18 0
fax: +49 (0) 69 / 27 22 18 22
web: www.compeople.de
Vorstand: Jürgen Wiesmaier
Aufsichtsratsvorsitzender: Christian Glanz
Sitz der Gesellschaft: Frankfurt/Main
Handelsregister Frankfurt HRB 56759
USt-IdNr. DE207665352
-------------------------------------------------------------
_______________________________________________
cross-project-issues-dev mailing list
cross-project-issues-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev