Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [birt-dev] Rules for PRs
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

Back to the top