Re: pull requests without description

[Nate Cole]

+1 for this extra exception by Attila.

On 4/11/18, 1:57 PM, "Attila Magyar"  wrote:

I agree, however I think there is one more exception, if the same patch 
should go into multiple branches, and one of them was already reviewed.

On 4/11/18, 5:37 PM, "Jonathan Hurley"  wrote:

Agreed - the reviews without +1's are most concerning. We can't be 
committing without proper reviews. I think the only exceptions are:

- reverts
- merges from a main branch to a feature branch

> On Apr 11, 2018, at 11:28 AM, Robert Levas  
wrote:
> 
> +1
> 
> On 4/11/18, 10:50 AM, "Doroszlai, Attila"  
wrote:
> 
>Hi all,
> 
>Can you please make sure to include meaningful description and test
>steps in your pull requests?  Please take some time to replace the
>placeholder text from the template (everything except the 2 
headings)
>to help others (reviewers, users) understand the context.
> 
>
https://github.com/apache/ambari/pulls?q=is%3Apr+in%3Abody+NOT+tested+NOT+proposed
> 
>On a related note, please merge only after the PR is approved by 
reviewers.
> 
>
https://github.com/apache/ambari/pulls?q=is%3Apr+is%3Amerged+review%3Anone+in%3Acomments+NOT+LGTM
> 
>-Attila
> 
> 
> 

Re: pull requests without description

[Attila Magyar]

I agree, however I think there is one more exception, if the same patch should 
go into multiple branches, and one of them was already reviewed.

On 4/11/18, 5:37 PM, "Jonathan Hurley"  wrote:

Agreed - the reviews without +1's are most concerning. We can't be 
committing without proper reviews. I think the only exceptions are:

- reverts
- merges from a main branch to a feature branch

> On Apr 11, 2018, at 11:28 AM, Robert Levas  wrote:
> 
> +1
> 
> On 4/11/18, 10:50 AM, "Doroszlai, Attila"  wrote:
> 
>Hi all,
> 
>Can you please make sure to include meaningful description and test
>steps in your pull requests?  Please take some time to replace the
>placeholder text from the template (everything except the 2 headings)
>to help others (reviewers, users) understand the context.
> 
>
https://github.com/apache/ambari/pulls?q=is%3Apr+in%3Abody+NOT+tested+NOT+proposed
> 
>On a related note, please merge only after the PR is approved by 
reviewers.
> 
>
https://github.com/apache/ambari/pulls?q=is%3Apr+is%3Amerged+review%3Anone+in%3Acomments+NOT+LGTM
> 
>-Attila
> 
> 
> 

Re: pull requests without description

[Vivek Ratnavel]

+1

I agree with Jonathan that commits without +1 from reviewers are most
concerning. This shouldn't be happening.

-Vivek Ratnavel

On Wed, Apr 11, 2018 at 8:37 AM, Jonathan Hurley 
wrote:

> Agreed - the reviews without +1's are most concerning. We can't be
> committing without proper reviews. I think the only exceptions are:
>
> - reverts
> - merges from a main branch to a feature branch
>
> > On Apr 11, 2018, at 11:28 AM, Robert Levas 
> wrote:
> >
> > +1
> >
> > On 4/11/18, 10:50 AM, "Doroszlai, Attila" 
> wrote:
> >
> >Hi all,
> >
> >Can you please make sure to include meaningful description and test
> >steps in your pull requests?  Please take some time to replace the
> >placeholder text from the template (everything except the 2 headings)
> >to help others (reviewers, users) understand the context.
> >
> >https://github.com/apache/ambari/pulls?q=is%3Apr+in%
> 3Abody+NOT+tested+NOT+proposed
> >
> >On a related note, please merge only after the PR is approved by
> reviewers.
> >
> >https://github.com/apache/ambari/pulls?q=is%3Apr+is%
> 3Amerged+review%3Anone+in%3Acomments+NOT+LGTM
> >
> >-Attila
> >
> >
> >
>
>

Re: pull requests without description

[Jonathan Hurley]

Agreed - the reviews without +1's are most concerning. We can't be committing 
without proper reviews. I think the only exceptions are:

- reverts
- merges from a main branch to a feature branch

> On Apr 11, 2018, at 11:28 AM, Robert Levas  wrote:
> 
> +1
> 
> On 4/11/18, 10:50 AM, "Doroszlai, Attila"  wrote:
> 
>Hi all,
> 
>Can you please make sure to include meaningful description and test
>steps in your pull requests?  Please take some time to replace the
>placeholder text from the template (everything except the 2 headings)
>to help others (reviewers, users) understand the context.
> 
>
> https://github.com/apache/ambari/pulls?q=is%3Apr+in%3Abody+NOT+tested+NOT+proposed
> 
>On a related note, please merge only after the PR is approved by reviewers.
> 
>
> https://github.com/apache/ambari/pulls?q=is%3Apr+is%3Amerged+review%3Anone+in%3Acomments+NOT+LGTM
> 
>-Attila
> 
> 
> 

Re: pull requests without description

[Robert Levas]

+1

On 4/11/18, 10:50 AM, "Doroszlai, Attila"  wrote:

Hi all,

Can you please make sure to include meaningful description and test
steps in your pull requests?  Please take some time to replace the
placeholder text from the template (everything except the 2 headings)
to help others (reviewers, users) understand the context.


https://github.com/apache/ambari/pulls?q=is%3Apr+in%3Abody+NOT+tested+NOT+proposed

On a related note, please merge only after the PR is approved by reviewers.


https://github.com/apache/ambari/pulls?q=is%3Apr+is%3Amerged+review%3Anone+in%3Acomments+NOT+LGTM

-Attila