Some notes before I forget:

1. Except if Infra (or I) missed something, GH "Rebase and Merge" does not 
always create a linear commit history. See the log of ofbiz-site after the
   "Rebase and Merge" I did for PR1. If commits happen beetwen it's does not 
deliver a linear commit history.
2. With our request to Infra at INFRA-19950, which was normally based on Matthieu's [1] 
link below, the "Squash and Merge" option is still available.
   AFAIK it does not rebase before squashing and merging. I though find it 
convenient in some cases. But we can also ask contributors to manually
   squash their commits before committing the lot again (always in a new PR?) 
and ask Infra to also remove this option. Opinions?

Jacques

Le 13/03/2020 à 14:48, Jacques Le Roux a écrit :
Hi All,

This is done, you may check with an open GH PR

We will now ask Infra to add GH Issues[1]. It needs again a PMC agreement.

[1] 
https://help.github.com/en/github/managing-your-work-on-github/creating-an-issue

Jacques

Le 10/03/2020 à 11:22, Jacques Le Roux a écrit :
The infra team requires a PMC decision for this change, see 
https://issues.apache.org/jira/browse/INFRA-19950

Jacques

Le 10/03/2020 à 10:57, Jacques Le Roux a écrit :
Le 09/03/2020 à 17:58, Mathieu Lirzin a écrit :
Hello,

The history of OFBiz trunk with the adoption of the Pull Request based
contribution process is getting less and less readable. Here is a
snippet of `git log --oneline --graph` demonstrating that:

--8<---------------cut here---------------start------------->8---
a3bcdc4cc3 * | Improved: Removes getSubContentWithPermCheck and getSubSub
            |/
8ec3886d7f * Fixed: Code refactoring to support groovy syntax (OFBIZ-1023
604a3bfa02 * Improved: Convert PartyInvitationService.xml from minilang t
5b8c89906c *   Merge pull request #36 from danwatford/ofbiz-11428-checkst
            |\
e665f9dc68 | * Improved: Set checkstyle to use LF line endings
e9ec4181ec * |   Merge pull request #17 from PierreSmits/Label-Cleanup
            |\ \
974b85f4ec | * | Improved: Cleanup HumanRes labels
c71a7ae06d | * | Improved: UI labels
c121ad6b9d * | |   Merge pull request #15 from PierreSmits/OFBIZ-10551
            |\ \ \
            | |_|/
            |/| |
58b0da26f5 | * | Improved: Remove unused labels from ProductUiLabels.xml
66aa76d7f7 * | |   Merge pull request #35 from danwatford/ofbiz-11418-doc
            |\ \ \
0ece441228 | * | | Fixed: Fixed line lengths in ModelFormFieldTest to adh
cfad407c48 * | | |   Merge pull request #34 from danwatford/ofbiz-11418-d
            |\ \ \ \
            | |/ / /
5640de4eba | * | | Documented: Documented use of field attribute paramete
--8<---------------cut here---------------end--------------->8---

I personnally think this is a huge issue because it makes analysing
history and chasing previously introduced bugs unecessary hard.

I would strongly recommend configuring OFBiz Github to require a linear
commit history when merging PR [1].

Thanks.

[1] 
https://help.github.com/en/github/administering-a-repository/requiring-a-linear-commit-history

PS: Even if I try to be polite, I am profoundly angry regarding the way
the PR contribution process has been adopted without any formal
community approval or listening to people having stated important
requirements that needed to be addressed before moving towards that new
contribution process.

Hi Mathieu,

If my opinion is worth telling, I was initially reluctant to use the PR process instead of the patch process. Because in general I prefer to control things, it's even sometimes a problem for me in real life. I must say it was more laziness which inclined me to PR.

I noticed that sometimes strange things happen when you use a PR. Consider this recent email for instance: https://markmail.org/message/amq5fj2dfma7pcbb.

There is only the files names, nothing about the changes. You have to get there to have the information https://github.com/apache/ofbiz-plugins/commit/4afe603/ not very convenient :/

Moreover if you look at the commit comment in related Jira (OFBIZ-10948), it 
says:

   <<Merge pull request #7 from priyasharma1/OFBIZ-10948 
<https://issues.apache.org/jira/browse/OFBIZ-10948>

   Improved: Convert DimensionServices.xml minilang to groovy (OFBIZ-10948 
<https://issues.apache.org/jira/browse/OFBIZ-10948>)>>

When I supposed it would be only the title and comment at https://github.com/apache/ofbiz-plugins/pull/7. Why is "Merge pull request #7 from priyasharma1/OFBIZ-10948 <https://issues.apache.org/jira/browse/OFBIZ-10948>" added, mystery!

For a reason I ignore (must be a reason, but I'll not try to understand) we did 
not get that.

Sorry for the digression, now about your point, I agree. I find strange that the default value of the GH (GitHub) merge button is not merge and rebase. I guess it's history and legacy... Using the squash value is also a good option when the merge passes and there are several commits, it simplifies things when reading emails.

Like Michael I had a look about the settings. The settings button is available 
on my fork but not on the official mirror Github repo.

This said you certainly saw this thread started by Pierre Smits: https://markmail.org/message/so7ljoqxzuq7jplz and the related wiki document https://cwiki.apache.org/confluence/display/OFBIZ/Contributing+via+Git+and+Github

The discussion ended with Michael's disagreement, as somehow yours, but nothing happened. So I think we should move ahead with your proposition and note the change in the wiki page. I have created https://issues.apache.org/jira/browse/INFRA-19950 for that. If somebody disagrees please speak now, even if it will always possible to revert the change.

We have also to maintain the related stuff which are also still WIP:

https://cwiki.apache.org/confluence/display/OFBIZ/Migration+from+Subversion+%28SVN%29+to+Git
https://issues.apache.org/jira/browse/OFBIZ-11268

Nothing happens magically, but it seems we are near the end about the migration 
from Subversion to Git process.

BTW, a question for you: do you think not having a linear commit history would have an impact on Bisect. I don't think so, just asking in case you have an experience with that.

Please don't be angry, it's not good for health. Just listen few minutes to 
https://www.youtube.com/watch?v=d-diB65scQU it generally helps ;)

Thanks for your proposition!

Jacques

Reply via email to