[aerogear-dev] Review process for aerogear PR's

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[aerogear-dev] Review process for aerogear PR's

Wojciech Trocki
Hi Aerogear Developers :)

Do you guys think that we can use github squash or merge option as default action for aerogear digger repos (or even every aerogear repository)? 

 Aerogear contribution guide doesn't state how tickets are reviewed etc. but I see that most of the opensource projects use squashing option after merging to master - this would still allow us to push as many commits as we want. Squashing would be performed before merge to master on github. This would make our git logs clean and would simplify releases etc.

I do not want to start some holly war here, but I would just like to keep things clean and avoid some commits to repo without actual meaning or simply incomplete. When checking changes on the master multiple commits can make things difficult. It would be hard to provide some release notes basing on that etc. 

Regards

--
Wojciech Trocki
Software Engineer, Red Hat Mobile


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [aerogear-dev] Review process for aerogear PR's

Wojciech Trocki
TL;DR - Suggested options:


​EXPLANATION:
 - Default option used for most of the cases
 - Use when you want to integrate changes with multiple commits 


On Tue, Dec 13, 2016 at 5:19 PM, Wojciech Trocki <[hidden email]> wrote:
Hi Aerogear Developers :)

Do you guys think that we can use github squash or merge option as default action for aerogear digger repos (or even every aerogear repository)? 

 Aerogear contribution guide doesn't state how tickets are reviewed etc. but I see that most of the opensource projects use squashing option after merging to master - this would still allow us to push as many commits as we want. Squashing would be performed before merge to master on github. This would make our git logs clean and would simplify releases etc.

I do not want to start some holly war here, but I would just like to keep things clean and avoid some commits to repo without actual meaning or simply incomplete. When checking changes on the master multiple commits can make things difficult. It would be hard to provide some release notes basing on that etc. 

Regards

--
Wojciech Trocki
Software Engineer, Red Hat Mobile




--
Wojciech Trocki
Software Engineer, Red Hat Mobile


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [aerogear-dev] Review process for aerogear PR's

Sebastien Blanc
Why don't keep following what is defined here https://aerogear.org/docs/guides/GitHubWorkflow/

On Tue, Dec 13, 2016 at 6:44 PM, Wojciech Trocki <[hidden email]> wrote:
TL;DR - Suggested options:


​EXPLANATION:
 - Default option used for most of the cases
 - Use when you want to integrate changes with multiple commits 


On Tue, Dec 13, 2016 at 5:19 PM, Wojciech Trocki <[hidden email]> wrote:
Hi Aerogear Developers :)

Do you guys think that we can use github squash or merge option as default action for aerogear digger repos (or even every aerogear repository)? 

 Aerogear contribution guide doesn't state how tickets are reviewed etc. but I see that most of the opensource projects use squashing option after merging to master - this would still allow us to push as many commits as we want. Squashing would be performed before merge to master on github. This would make our git logs clean and would simplify releases etc.

I do not want to start some holly war here, but I would just like to keep things clean and avoid some commits to repo without actual meaning or simply incomplete. When checking changes on the master multiple commits can make things difficult. It would be hard to provide some release notes basing on that etc. 

Regards

--
Wojciech Trocki
Software Engineer, Red Hat Mobile




--
Wojciech Trocki
Software Engineer, Red Hat Mobile


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [aerogear-dev] Review process for aerogear PR's

Wojciech Trocki
Thanks Sebi. I wasn't aware of this document. 
It actually matches my suggestion. 
Recently github added 2 different options for PR's so suggestion here is to also use this options more to automate merges.
Git workflow should not be affected.


On Tue, Dec 13, 2016 at 5:54 PM, Sebastien Blanc <[hidden email]> wrote:
Why don't keep following what is defined here https://aerogear.org/docs/guides/GitHubWorkflow/

On Tue, Dec 13, 2016 at 6:44 PM, Wojciech Trocki <[hidden email]> wrote:
TL;DR - Suggested options:


​EXPLANATION:
 - Default option used for most of the cases
 - Use when you want to integrate changes with multiple commits 


On Tue, Dec 13, 2016 at 5:19 PM, Wojciech Trocki <[hidden email]> wrote:
Hi Aerogear Developers :)

Do you guys think that we can use github squash or merge option as default action for aerogear digger repos (or even every aerogear repository)? 

 Aerogear contribution guide doesn't state how tickets are reviewed etc. but I see that most of the opensource projects use squashing option after merging to master - this would still allow us to push as many commits as we want. Squashing would be performed before merge to master on github. This would make our git logs clean and would simplify releases etc.

I do not want to start some holly war here, but I would just like to keep things clean and avoid some commits to repo without actual meaning or simply incomplete. When checking changes on the master multiple commits can make things difficult. It would be hard to provide some release notes basing on that etc. 

Regards

--
Wojciech Trocki
Software Engineer, Red Hat Mobile




--
Wojciech Trocki
Software Engineer, Red Hat Mobile


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--
Wojciech Trocki
Software Engineer, Red Hat Mobile


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [aerogear-dev] Review process for aerogear PR's

Matthias Wessendorf
for large PRs, we tend to have lot's of commits for review, and tend to manually squash/rebase before final push

On Tue, Dec 13, 2016 at 7:20 PM, Wojciech Trocki <[hidden email]> wrote:
Thanks Sebi. I wasn't aware of this document. 
It actually matches my suggestion. 
Recently github added 2 different options for PR's so suggestion here is to also use this options more to automate merges.
Git workflow should not be affected.


On Tue, Dec 13, 2016 at 5:54 PM, Sebastien Blanc <[hidden email]> wrote:
Why don't keep following what is defined here https://aerogear.org/docs/guides/GitHubWorkflow/

On Tue, Dec 13, 2016 at 6:44 PM, Wojciech Trocki <[hidden email]> wrote:
TL;DR - Suggested options:


​EXPLANATION:
 - Default option used for most of the cases
 - Use when you want to integrate changes with multiple commits 


On Tue, Dec 13, 2016 at 5:19 PM, Wojciech Trocki <[hidden email]> wrote:
Hi Aerogear Developers :)

Do you guys think that we can use github squash or merge option as default action for aerogear digger repos (or even every aerogear repository)? 

 Aerogear contribution guide doesn't state how tickets are reviewed etc. but I see that most of the opensource projects use squashing option after merging to master - this would still allow us to push as many commits as we want. Squashing would be performed before merge to master on github. This would make our git logs clean and would simplify releases etc.

I do not want to start some holly war here, but I would just like to keep things clean and avoid some commits to repo without actual meaning or simply incomplete. When checking changes on the master multiple commits can make things difficult. It would be hard to provide some release notes basing on that etc. 

Regards

--
Wojciech Trocki
Software Engineer, Red Hat Mobile




--
Wojciech Trocki
Software Engineer, Red Hat Mobile


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--
Wojciech Trocki
Software Engineer, Red Hat Mobile


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--

_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [aerogear-dev] Review process for aerogear PR's

Corinne Krych
+1 Wojciech 
Rather than saying don't use merge button, let's make it match the recommendation in contributor guide.

On 13 December 2016 at 19:29, Matthias Wessendorf <[hidden email]> wrote:
for large PRs, we tend to have lot's of commits for review, and tend to manually squash/rebase before final push

On Tue, Dec 13, 2016 at 7:20 PM, Wojciech Trocki <[hidden email]> wrote:
Thanks Sebi. I wasn't aware of this document. 
It actually matches my suggestion. 
Recently github added 2 different options for PR's so suggestion here is to also use this options more to automate merges.
Git workflow should not be affected.


On Tue, Dec 13, 2016 at 5:54 PM, Sebastien Blanc <[hidden email]> wrote:
Why don't keep following what is defined here https://aerogear.org/docs/guides/GitHubWorkflow/

On Tue, Dec 13, 2016 at 6:44 PM, Wojciech Trocki <[hidden email]> wrote:
TL;DR - Suggested options:


​EXPLANATION:
 - Default option used for most of the cases
 - Use when you want to integrate changes with multiple commits 


On Tue, Dec 13, 2016 at 5:19 PM, Wojciech Trocki <[hidden email]> wrote:
Hi Aerogear Developers :)

Do you guys think that we can use github squash or merge option as default action for aerogear digger repos (or even every aerogear repository)? 

 Aerogear contribution guide doesn't state how tickets are reviewed etc. but I see that most of the opensource projects use squashing option after merging to master - this would still allow us to push as many commits as we want. Squashing would be performed before merge to master on github. This would make our git logs clean and would simplify releases etc.

I do not want to start some holly war here, but I would just like to keep things clean and avoid some commits to repo without actual meaning or simply incomplete. When checking changes on the master multiple commits can make things difficult. It would be hard to provide some release notes basing on that etc. 

Regards

--
Wojciech Trocki
Software Engineer, Red Hat Mobile




--
Wojciech Trocki
Software Engineer, Red Hat Mobile


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--
Wojciech Trocki
Software Engineer, Red Hat Mobile


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--

_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Loading...