You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Martin Liesenberg <ma...@gmail.com> on 2016/02/19 18:19:26 UTC
Introducing a PR template
Based on the recent discussion in the email thread 'Extending and improving
our "How to contribute" page', I propose to introduce the following
template for PRs
----
Thanks for contributing to Apache Flink, before you open your PR please
kindly take into consideration the following check list.
Once you are sure, all items on the list can be checked, feel free to open
your PR. For more information please refer to the How To Contribute guide
linked above.
### General
- [ ] Is there an associated JIRA issue
- [ ] This PR addresses includes a single change
- [ ] New functionality is covered by tests
- [ ] Documentation is up to date
### Code health
- [ ] Tests pass (`mvn test`)
- [ ] Build passes (`mvn install`)
- [ ] Check style passes (`mvn verfiy`)
- [ ] JavaDoc for new `public` methods has been added
---
The intended effects would be:
- reduce friction in the PR process created by basic oversights such as
checkstyle violations or missing tests
- provide a helping hand for new contributors
I tried to condense the suggestion on the mailing list to make it not too
long and intimidating but at the same time cover the most important points.
Looking forward to your input.
Best regards
martin
Re: Introducing a PR template
Posted by Martin Liesenberg <ma...@gmail.com>.
I guess with the upcoming 1.0 release there's heaps of more urgent things
on your plate, nevertheless just a quick update: I added a JIRA ticket [1]
and opened a PR [2] with the changes.
best regards
martin
[1] https://issues.apache.org/jira/browse/FLINK-3529
[2] https://github.com/apache/flink/pull/1729
Martin Liesenberg <ma...@gmail.com> schrieb am Di., 23. Feb.
2016 um 00:12 Uhr:
> sure. will take that into account. Thanks for the input.
>
> best regards
> martin
>
> Maximilian Michels <mx...@apache.org> schrieb am So., 21. Feb. 2016 um
> 13:49 Uhr:
>
>> Hi Martin,
>>
>> Thanks for the proposal. This is a great idea and will help new
>> contributors.
>>
>> How about having three sections and less check boxes? I think checking
>> all those boxes will get announcing for regular contributors.
>>
>> [ ] Pull Request
>> - JIRA issue associated
>> - Pull request only addresses one issue
>> - Meaningful commit message
>>
>> [ ] Documentation
>> - New documentation added
>> - Old documentation updated
>> - JavaDoc for public methods
>>
>> [ ] Tests
>> - Tests added for new functionality
>> - Executed "mvn clean verify" or built on Travis
>>
>>
>> Cheers,
>> Max
>>
>> On Sat, Feb 20, 2016 at 3:21 AM, Jamie Grier <ja...@data-artisans.com>
>> wrote:
>> > +1
>> >
>> > On Fri, Feb 19, 2016 at 9:30 AM, Fabian Hueske <fh...@gmail.com>
>> wrote:
>> >
>> >> Hi Martin,
>> >>
>> >> "mvn install" does include the goals "test" and "verify".
>> >> In fact, "verify" is enough, because "install" does only copy the
>> results
>> >> into the local Maven repository (~/.m2/repository).
>> >>
>> >> So I think
>> >> - [ ] Tests pass (`mvn test`)
>> >> - [ ] Build passes (`mvn install`)
>> >> - [ ] Check style passes (`mvn verfiy`)
>> >>
>> >> can be condensed to
>> >> - [ ] Build passes (`mvn clean verify`)
>> >>
>> >> Otherwise, this looks good, IMO :-)
>> >>
>> >> Thanks, Fabian
>> >>
>> >> 2016-02-19 18:19 GMT+01:00 Martin Liesenberg <
>> martin.liesenberg@gmail.com
>> >> >:
>> >>
>> >> > Based on the recent discussion in the email thread 'Extending and
>> >> improving
>> >> > our "How to contribute" page', I propose to introduce the following
>> >> > template for PRs
>> >> >
>> >> > ----
>> >> > Thanks for contributing to Apache Flink, before you open your PR
>> please
>> >> > kindly take into consideration the following check list.
>> >> > Once you are sure, all items on the list can be checked, feel free to
>> >> open
>> >> > your PR. For more information please refer to the How To Contribute
>> guide
>> >> > linked above.
>> >> >
>> >> > ### General
>> >> > - [ ] Is there an associated JIRA issue
>> >> > - [ ] This PR addresses includes a single change
>> >> > - [ ] New functionality is covered by tests
>> >> > - [ ] Documentation is up to date
>> >> >
>> >> > ### Code health
>> >> > - [ ] Tests pass (`mvn test`)
>> >> > - [ ] Build passes (`mvn install`)
>> >> > - [ ] Check style passes (`mvn verfiy`)
>> >> > - [ ] JavaDoc for new `public` methods has been added
>> >> > ---
>> >> >
>> >> >
>> >> > The intended effects would be:
>> >> > - reduce friction in the PR process created by basic oversights such
>> as
>> >> > checkstyle violations or missing tests
>> >> > - provide a helping hand for new contributors
>> >> >
>> >> > I tried to condense the suggestion on the mailing list to make it
>> not too
>> >> > long and intimidating but at the same time cover the most important
>> >> points.
>> >> >
>> >> > Looking forward to your input.
>> >> > Best regards
>> >> > martin
>> >> >
>> >>
>> >
>> >
>> >
>> > --
>> >
>> > Jamie Grier
>> > data Artisans, Director of Applications Engineering
>> > @jamiegrier <https://twitter.com/jamiegrier>
>> > jamie@data-artisans.com
>>
>
Re: Introducing a PR template
Posted by Martin Liesenberg <ma...@gmail.com>.
sure. will take that into account. Thanks for the input.
best regards
martin
Maximilian Michels <mx...@apache.org> schrieb am So., 21. Feb. 2016 um
13:49 Uhr:
> Hi Martin,
>
> Thanks for the proposal. This is a great idea and will help new
> contributors.
>
> How about having three sections and less check boxes? I think checking
> all those boxes will get announcing for regular contributors.
>
> [ ] Pull Request
> - JIRA issue associated
> - Pull request only addresses one issue
> - Meaningful commit message
>
> [ ] Documentation
> - New documentation added
> - Old documentation updated
> - JavaDoc for public methods
>
> [ ] Tests
> - Tests added for new functionality
> - Executed "mvn clean verify" or built on Travis
>
>
> Cheers,
> Max
>
> On Sat, Feb 20, 2016 at 3:21 AM, Jamie Grier <ja...@data-artisans.com>
> wrote:
> > +1
> >
> > On Fri, Feb 19, 2016 at 9:30 AM, Fabian Hueske <fh...@gmail.com>
> wrote:
> >
> >> Hi Martin,
> >>
> >> "mvn install" does include the goals "test" and "verify".
> >> In fact, "verify" is enough, because "install" does only copy the
> results
> >> into the local Maven repository (~/.m2/repository).
> >>
> >> So I think
> >> - [ ] Tests pass (`mvn test`)
> >> - [ ] Build passes (`mvn install`)
> >> - [ ] Check style passes (`mvn verfiy`)
> >>
> >> can be condensed to
> >> - [ ] Build passes (`mvn clean verify`)
> >>
> >> Otherwise, this looks good, IMO :-)
> >>
> >> Thanks, Fabian
> >>
> >> 2016-02-19 18:19 GMT+01:00 Martin Liesenberg <
> martin.liesenberg@gmail.com
> >> >:
> >>
> >> > Based on the recent discussion in the email thread 'Extending and
> >> improving
> >> > our "How to contribute" page', I propose to introduce the following
> >> > template for PRs
> >> >
> >> > ----
> >> > Thanks for contributing to Apache Flink, before you open your PR
> please
> >> > kindly take into consideration the following check list.
> >> > Once you are sure, all items on the list can be checked, feel free to
> >> open
> >> > your PR. For more information please refer to the How To Contribute
> guide
> >> > linked above.
> >> >
> >> > ### General
> >> > - [ ] Is there an associated JIRA issue
> >> > - [ ] This PR addresses includes a single change
> >> > - [ ] New functionality is covered by tests
> >> > - [ ] Documentation is up to date
> >> >
> >> > ### Code health
> >> > - [ ] Tests pass (`mvn test`)
> >> > - [ ] Build passes (`mvn install`)
> >> > - [ ] Check style passes (`mvn verfiy`)
> >> > - [ ] JavaDoc for new `public` methods has been added
> >> > ---
> >> >
> >> >
> >> > The intended effects would be:
> >> > - reduce friction in the PR process created by basic oversights such
> as
> >> > checkstyle violations or missing tests
> >> > - provide a helping hand for new contributors
> >> >
> >> > I tried to condense the suggestion on the mailing list to make it not
> too
> >> > long and intimidating but at the same time cover the most important
> >> points.
> >> >
> >> > Looking forward to your input.
> >> > Best regards
> >> > martin
> >> >
> >>
> >
> >
> >
> > --
> >
> > Jamie Grier
> > data Artisans, Director of Applications Engineering
> > @jamiegrier <https://twitter.com/jamiegrier>
> > jamie@data-artisans.com
>
Re: Introducing a PR template
Posted by Maximilian Michels <mx...@apache.org>.
Hi Martin,
Thanks for the proposal. This is a great idea and will help new contributors.
How about having three sections and less check boxes? I think checking
all those boxes will get announcing for regular contributors.
[ ] Pull Request
- JIRA issue associated
- Pull request only addresses one issue
- Meaningful commit message
[ ] Documentation
- New documentation added
- Old documentation updated
- JavaDoc for public methods
[ ] Tests
- Tests added for new functionality
- Executed "mvn clean verify" or built on Travis
Cheers,
Max
On Sat, Feb 20, 2016 at 3:21 AM, Jamie Grier <ja...@data-artisans.com> wrote:
> +1
>
> On Fri, Feb 19, 2016 at 9:30 AM, Fabian Hueske <fh...@gmail.com> wrote:
>
>> Hi Martin,
>>
>> "mvn install" does include the goals "test" and "verify".
>> In fact, "verify" is enough, because "install" does only copy the results
>> into the local Maven repository (~/.m2/repository).
>>
>> So I think
>> - [ ] Tests pass (`mvn test`)
>> - [ ] Build passes (`mvn install`)
>> - [ ] Check style passes (`mvn verfiy`)
>>
>> can be condensed to
>> - [ ] Build passes (`mvn clean verify`)
>>
>> Otherwise, this looks good, IMO :-)
>>
>> Thanks, Fabian
>>
>> 2016-02-19 18:19 GMT+01:00 Martin Liesenberg <martin.liesenberg@gmail.com
>> >:
>>
>> > Based on the recent discussion in the email thread 'Extending and
>> improving
>> > our "How to contribute" page', I propose to introduce the following
>> > template for PRs
>> >
>> > ----
>> > Thanks for contributing to Apache Flink, before you open your PR please
>> > kindly take into consideration the following check list.
>> > Once you are sure, all items on the list can be checked, feel free to
>> open
>> > your PR. For more information please refer to the How To Contribute guide
>> > linked above.
>> >
>> > ### General
>> > - [ ] Is there an associated JIRA issue
>> > - [ ] This PR addresses includes a single change
>> > - [ ] New functionality is covered by tests
>> > - [ ] Documentation is up to date
>> >
>> > ### Code health
>> > - [ ] Tests pass (`mvn test`)
>> > - [ ] Build passes (`mvn install`)
>> > - [ ] Check style passes (`mvn verfiy`)
>> > - [ ] JavaDoc for new `public` methods has been added
>> > ---
>> >
>> >
>> > The intended effects would be:
>> > - reduce friction in the PR process created by basic oversights such as
>> > checkstyle violations or missing tests
>> > - provide a helping hand for new contributors
>> >
>> > I tried to condense the suggestion on the mailing list to make it not too
>> > long and intimidating but at the same time cover the most important
>> points.
>> >
>> > Looking forward to your input.
>> > Best regards
>> > martin
>> >
>>
>
>
>
> --
>
> Jamie Grier
> data Artisans, Director of Applications Engineering
> @jamiegrier <https://twitter.com/jamiegrier>
> jamie@data-artisans.com
Re: Introducing a PR template
Posted by Jamie Grier <ja...@data-artisans.com>.
+1
On Fri, Feb 19, 2016 at 9:30 AM, Fabian Hueske <fh...@gmail.com> wrote:
> Hi Martin,
>
> "mvn install" does include the goals "test" and "verify".
> In fact, "verify" is enough, because "install" does only copy the results
> into the local Maven repository (~/.m2/repository).
>
> So I think
> - [ ] Tests pass (`mvn test`)
> - [ ] Build passes (`mvn install`)
> - [ ] Check style passes (`mvn verfiy`)
>
> can be condensed to
> - [ ] Build passes (`mvn clean verify`)
>
> Otherwise, this looks good, IMO :-)
>
> Thanks, Fabian
>
> 2016-02-19 18:19 GMT+01:00 Martin Liesenberg <martin.liesenberg@gmail.com
> >:
>
> > Based on the recent discussion in the email thread 'Extending and
> improving
> > our "How to contribute" page', I propose to introduce the following
> > template for PRs
> >
> > ----
> > Thanks for contributing to Apache Flink, before you open your PR please
> > kindly take into consideration the following check list.
> > Once you are sure, all items on the list can be checked, feel free to
> open
> > your PR. For more information please refer to the How To Contribute guide
> > linked above.
> >
> > ### General
> > - [ ] Is there an associated JIRA issue
> > - [ ] This PR addresses includes a single change
> > - [ ] New functionality is covered by tests
> > - [ ] Documentation is up to date
> >
> > ### Code health
> > - [ ] Tests pass (`mvn test`)
> > - [ ] Build passes (`mvn install`)
> > - [ ] Check style passes (`mvn verfiy`)
> > - [ ] JavaDoc for new `public` methods has been added
> > ---
> >
> >
> > The intended effects would be:
> > - reduce friction in the PR process created by basic oversights such as
> > checkstyle violations or missing tests
> > - provide a helping hand for new contributors
> >
> > I tried to condense the suggestion on the mailing list to make it not too
> > long and intimidating but at the same time cover the most important
> points.
> >
> > Looking forward to your input.
> > Best regards
> > martin
> >
>
--
Jamie Grier
data Artisans, Director of Applications Engineering
@jamiegrier <https://twitter.com/jamiegrier>
jamie@data-artisans.com
Re: Introducing a PR template
Posted by Fabian Hueske <fh...@gmail.com>.
Hi Martin,
"mvn install" does include the goals "test" and "verify".
In fact, "verify" is enough, because "install" does only copy the results
into the local Maven repository (~/.m2/repository).
So I think
- [ ] Tests pass (`mvn test`)
- [ ] Build passes (`mvn install`)
- [ ] Check style passes (`mvn verfiy`)
can be condensed to
- [ ] Build passes (`mvn clean verify`)
Otherwise, this looks good, IMO :-)
Thanks, Fabian
2016-02-19 18:19 GMT+01:00 Martin Liesenberg <ma...@gmail.com>:
> Based on the recent discussion in the email thread 'Extending and improving
> our "How to contribute" page', I propose to introduce the following
> template for PRs
>
> ----
> Thanks for contributing to Apache Flink, before you open your PR please
> kindly take into consideration the following check list.
> Once you are sure, all items on the list can be checked, feel free to open
> your PR. For more information please refer to the How To Contribute guide
> linked above.
>
> ### General
> - [ ] Is there an associated JIRA issue
> - [ ] This PR addresses includes a single change
> - [ ] New functionality is covered by tests
> - [ ] Documentation is up to date
>
> ### Code health
> - [ ] Tests pass (`mvn test`)
> - [ ] Build passes (`mvn install`)
> - [ ] Check style passes (`mvn verfiy`)
> - [ ] JavaDoc for new `public` methods has been added
> ---
>
>
> The intended effects would be:
> - reduce friction in the PR process created by basic oversights such as
> checkstyle violations or missing tests
> - provide a helping hand for new contributors
>
> I tried to condense the suggestion on the mailing list to make it not too
> long and intimidating but at the same time cover the most important points.
>
> Looking forward to your input.
> Best regards
> martin
>