Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [birt-dev] Rules for PRs
Yes, the ECA checker that the Eclipse Foundation webmaster installs on our Git repositories checks for both a current ECA and for the signed-off-by requirement. Note that BIRT project committers don't need to add the signed-off-by (I tend to add it to all of my commits just as a matter of practice).

Regarding the 1KLOC IP review check requirement, it's expressed in both the committer guidelines and the handbook.

I tend to agree with your comments regarding generated code. However, in practice it tends to be committers that generate the code and the IP Team review requirement doesn't apply to committers doing day-to-day work on a project.

Asking a contributor to break up changes into manageable functionally-complete chunks for review purposes is completely reasonable. Please do not ask contributors to arbitrarily break up contributions to avoid having to deal with the IP Team.

Send a note to emo@eclipse with a pointer to the problematic pull request and I'll have a look.

Wayne



On Sun, Mar 7, 2021 at 1:40 PM Alexander Lehmann <a.lehm@xxxxxx> wrote:
Hi,

ahh, I mistook the signing that can be enforced by the GitHub branch protection with the "signed off by" that the ECA requires. The latter is then enforced by the ECA-check already?

About the 1KLOC policy: Can you provide a link to that? Couldn't find one by myself. I hope there is an exclusion for generated code since it is not affected by IP rights (at least not in Germany). The example we currently face (one of my pull requests) is such a case (settings files generated by m2e).

For contributions made by a human I totally agree; I prefer not having to review 1000+ lines in one commit and if I face such a case I will request the contributor to break up the changes.

Best regards
Alex

Am So., 7. März 2021 um 18:46 Uhr schrieb Wayne Beaton <wayne.beaton@xxxxxxxxxxxxxxxxxxxxxx>:
The Eclipse Foundation requires that all contributions be signed off by contributors who have signed the Eclipse Contributor Agreement (that is, they need to include a "Signed-off-by" field in the footer). This is different from signing commits. I'm actually curious to see how requiring that commits be signed actually works in practice. I'm concerned, though, that requiring it would be yet another barrier and we should be all about lowering barriers at this point.

Regarding commit size... Per the IP Policy, any commit of more than 1KLOC needs to be reviewed by the IP Team. I know that there are some committers who encourage contributors to chop up their contributions to keep the commit size down to avoid this requirement. Please don't do this.

Having said all that, IMHO, it is completely unreasonable to expect a committer to review a very large contribution. Further, it is better for everybody, committers and contributors, to push work in incremental chunks (reviewing is easier, and it's easier for others to keep up with the changes). Massive code dumps just make it hard for others to collaborate.

Wayne

On Sun, Mar 7, 2021 at 9:13 AM Wim Jongman <wim.jongman@xxxxxxxxx> wrote:
Yes, I think that a review is required for any substantial commit. I think a committer has to request a review. Can this be done in GH?

Signed commits are not needed IMO because of the checks the foundation already requires.

Cheers,

Wim

On Sun, Mar 7, 2021 at 9:52 AM Alexander Lehmann <a.lehm@xxxxxx> wrote:
Hi all,

should we agree on some rules for PRs? Alexander F. mentioned for example an upper bound on the LoC per PR. I would like to suggest that a review must be made before a PR can be merged. Should we also require commits to be signed? This can both be accomplished throught a GitHub branch protection rule [1].


Best regards
Alex
_______________________________________________
birt-dev mailing list
birt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/birt-dev
_______________________________________________
birt-dev mailing list
birt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/birt-dev


--

Wayne Beaton

Director of Open Source Projects | Eclipse Foundation

_______________________________________________
birt-dev mailing list
birt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/birt-dev
_______________________________________________
birt-dev mailing list
birt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/birt-dev


--

Wayne Beaton

Director of Open Source Projects | Eclipse Foundation

Back to the top