[cross-project-issues-dev] SimRel - direct push to master has been disabled

Discussion:

Frederic Gurr

2018-11-12 17:34:42 UTC

Hi,

As discussed before, direct push to master for the SimRel aggregation
build repository
(https://git.eclipse.org/r/simrel/org.eclipse.simrel.build) has been
disabled. So, all future commits have to go through a Gerrit review and
should get a +1 from the CI server before being merged.

The reasoning behind this is, that commits directly pushed to master
caused build failures and delays in the past.

Please let me know, if you have any questions or concerns.

Regards,

Fred

--
Frederic Gurr
Release Engineer | Eclipse Foundation Europe GmbH

Annastr. 46, D-64673 Zwingenberg
Handelsregister: Darmstadt HRB 92821
Managing Directors: Ralph Mueller, Mike Milinkovich, Chris Laroque

Ed Willink

2018-11-12 21:04:15 UTC

Permalink

Hi

Sorry I must have missed something. My no doubt flawed recollection was
that we were assured (again) that direct push would continue to be allowed.

I certainly use it every time since I see no other way to do it.

I push to Gerrit, check for build success, then Push to master.

Given that the bulk of failures are surely due to those updating magic
'latest' contributions without any commit, why change the rules for more
diligent users?

Regards

Ed Willink

Post by Frederic Gurr
Hi,
As discussed before, direct push to master for the SimRel aggregation
build repository
(https://git.eclipse.org/r/simrel/org.eclipse.simrel.build) has been
disabled. So, all future commits have to go through a Gerrit review and
should get a +1 from the CI server before being merged.
The reasoning behind this is, that commits directly pushed to master
caused build failures and delays in the past.
Please let me know, if you have any questions or concerns.
Regards,
Fred

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Ed Merks

2018-11-13 05:46:33 UTC

Permalink

Ed,

Normally you would use the Gerrit review link to finish the processing.Â
E.g., for the last commit I used this link:

Â https://git.eclipse.org/r/#/c/132049/

After the initial commit, I waited for the build to finish so that CI
Bot (one of the automatic reviewers) adds a +1.Â That takes about 5
minutes. Then I used the Reply...Â (or the Review +2 button, which will
be there after the successful build) to make it possible to "Submit" the
changes to master, i.e., the Submit button will be there once all the
reviewers (CI Bot and you) have voted the changes up to the necessary level.

So the chain of events looks like this in the review:

After submitting, when I do a pull on the repo, my changes are pulled
and the repo is up-to-date (no longer one commit behind master).

I believe a non-dilegent user could remove CI Bot from the review to
submit their changes even when those did not pass the aggregation build
but it appears to me that there really is no good reason to allow direct
push to master as a way to completely bypass the CI Bot review.

Regards,
Ed

Post by Ed Willink
Hi
Sorry I must have missed something. My no doubt flawed recollection
was that we were assured (again) that direct push would continue to be
allowed.
I certainly use it every time since I see no other way to do it.
I push to Gerrit, check for build success, then Push to master.
Given that the bulk of failures are surely due to those updating magic
'latest' contributions without any commit, why change the rules for
more diligent users?
Â Â Â Regards
Â Â Â Â Â Â Ed Willink

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
_______________________________________________
cross-project-issues-dev mailing list
To change your delivery options, retrieve your password, or
unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/cross-project-issues-dev

Ed Willink

2018-11-13 08:38:48 UTC

Permalink

Post by Ed Merks
Ed,
Normally you would use the Gerrit review link to finish the
https://git.eclipse.org/r/#/c/132049/
After the initial commit, I waited for the build to finish so that CI
Bot (one of the automatic reviewers) adds a +1.Â That takes about 5
minutes. Then I used the Reply...Â (or the Review +2 button, which
will be there after the successful build) to make it possible to
"Submit" the changes to master, i.e., the Submit button will be there
once all the reviewers (CI Bot and you) have voted the changes up to
the necessary level.
After submitting, when I do a pull on the repo, my changes are pulled
and the repo is up-to-date (no longer one commit behind master).
I believe a non-dilegent user could remove CI Bot from the review to
submit their changes even when those did not pass the aggregation
build but it appears to me that there really is no good reason to
allow direct push to master as a way to completely bypass the CI Bot
review.
Regards,
Ed

Post by Ed Willink
Hi
Sorry I must have missed something. My no doubt flawed recollection
was that we were assured (again) that direct push would continue to
be allowed.
I certainly use it every time since I see no other way to do it.
I push to Gerrit, check for build success, then Push to master.
Given that the bulk of failures are surely due to those updating
magic 'latest' contributions without any commit, why change the rules
for more diligent users?
Â Â Â Regards
Â Â Â Â Â Â Ed Willink

_______________________________________________
cross-project-issues-dev mailing list
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/cross-project-issues-dev

Jonah Graham

2018-11-13 08:55:15 UTC

Permalink

Hi Ed,

If you want you can do a "Rebase" in the Gerrit UI before doing the Submit.

Alternatively, and what I personally prefer is to set gerrit's merge
strategy to either Fast forward only, or rebase. The Eclipse platform uses
fast forward only. This prevents any merge commits and ensures that the
gerrit tests the same version as is pushed. The merge strategy is a admin
preference on the repo, so something that webmaster/Fred would have to
change IIRC.

HTH,
Jonah

~~~
Jonah Graham
Kichwa Coders Ltd.
www.kichwacoders.com

Hi
Ah! It never occurred to me that "Reply" meant "Commit" / "Push" / "Merge"
or even "Review". I saw only "Cherry Pick" / " Cancel" which did not match
my requirements. What a truly awful UI.
What if I don't want to "Merge" since it leads to double history entries
that on normal projects are difficult to disentangle? Ok, SimRel commits
are highly orthogonal. But Push gives a nice clean history and forces a
Rebase that just occasionally Merge can fail to replicate. IMHO if a Merge
is necessary the Gerrit Review should iterate.
Regards
Ed Willink
Ed,
Normally you would use the Gerrit review link to finish the processing.
https://git.eclipse.org/r/#/c/132049/
After the initial commit, I waited for the build to finish so that CI Bot
(one of the automatic reviewers) adds a +1. That takes about 5 minutes.
Then I used the Reply... (or the Review +2 button, which will be there
after the successful build) to make it possible to "Submit" the changes to
master, i.e., the Submit button will be there once all the reviewers (CI
Bot and you) have voted the changes up to the necessary level.
After submitting, when I do a pull on the repo, my changes are pulled and
the repo is up-to-date (no longer one commit behind master).
I believe a non-dilegent user could remove CI Bot from the review to
submit their changes even when those did not pass the aggregation build but
it appears to me that there really is no good reason to allow direct push
to master as a way to completely bypass the CI Bot review.
Regards,
Ed
Hi
Sorry I must have missed something. My no doubt flawed recollection was
that we were assured (again) that direct push would continue to be allowed.
I certainly use it every time since I see no other way to do it.
I push to Gerrit, check for build success, then Push to master.
Given that the bulk of failures are surely due to those updating magic
'latest' contributions without any commit, why change the rules for more
diligent users?
Regards
Ed Willink
Hi,
As discussed before, direct push to master for the SimRel aggregation
build repository
(https://git.eclipse.org/r/simrel/org.eclipse.simrel.build) has been
disabled. So, all future commits have to go through a Gerrit review and
should get a +1 from the CI server before being merged.
The reasoning behind this is, that commits directly pushed to master
caused build failures and delays in the past.
Please let me know, if you have any questions or concerns.
Regards,
Fred
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
_______________________________________________
cross-project-issues-dev mailing list
To change your delivery options, retrieve your password, or unsubscribe
from this list, visit
https://www.eclipse.org/mailman/listinfo/cross-project-issues-dev
_______________________________________________
To change your delivery options, retrieve your password, or unsubscribe from this list, visithttps://www.eclipse.org/mailman/listinfo/cross-project-issues-dev
<https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient> Virus-free.
www.avast.com
<https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient>
<#m_-916945580506975019_DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
_______________________________________________
cross-project-issues-dev mailing list
To change your delivery options, retrieve your password, or unsubscribe
from this list, visit
https://www.eclipse.org/mailman/listinfo/cross-project-issues-dev

Ed Merks

2018-11-13 09:10:38 UTC

Permalink

Jonah,

+1 for fast-forward-only

In principle that could force one to rebase and then kick off another CI
Bot verification job, but that's not so likely to occur much in practice.

Cheers,
Ed

Post by Jonah Graham
Hi Ed,
If you want you can do a "Rebase" in the Gerrit UI before doing the Submit.
Alternatively, and what I personally prefer is to set gerrit's merge
strategy to either Fast forward only, or rebase. The Eclipse platform
uses fast forward only. This prevents any merge commits and ensures
that the gerrit tests the same version as is pushed. The merge
strategy is a admin preference on the repo, so something that
webmaster/Fred would have to change IIRC.
HTH,
Jonah
~~~
Jonah Graham
Kichwa Coders Ltd.
www.kichwacoders.com <http://www.kichwacoders.com>
Hi
Ah! It never occurred to me that "Reply" meant "Commit" / "Push" /
"Merge" or even "Review". I saw only "Cherry Pick" / " Cancel"
which did not match my requirements. What a truly awful UI.
What if I don't want to "Merge" since it leads to double history
entries that on normal projects are difficult to disentangle? Ok,
SimRel commits are highly orthogonal. But Push gives a nice clean
history and forces a Rebase that just occasionally Merge can fail
to replicate. IMHO if a Merge is necessary the Gerrit Review
should iterate.
Â Â Â Regards
Â Â Â Â Â Â Ed Willink

Post by Ed Merks
Ed,
Normally you would use the Gerrit review link to finish the
https://git.eclipse.org/r/#/c/132049/
After the initial commit, I waited for the build to finish so
that CI Bot (one of the automatic reviewers) adds a +1.Â That
takes about 5 minutes. Then I used the Reply...Â (or the Review
+2 button, which will be there after the successful build) to
make it possible to "Submit" the changes to master, i.e., the
Submit button will be there once all the reviewers (CI Bot and
you) have voted the changes up to the necessary level.
After submitting, when I do a pull on the repo, my changes are
pulled and the repo is up-to-date (no longer one commit behind
master).
I believe a non-dilegent user could remove CI Bot from the review
to submit their changes even when those did not pass the
aggregation build but it appears to me that there really is no
good reason to allow direct push to master as a way to completely
bypass the CI Bot review.
Regards,
Ed

Post by Ed Willink
Hi
Sorry I must have missed something. My no doubt flawed
recollection was that we were assured (again) that direct push
would continue to be allowed.
I certainly use it every time since I see no other way to do it.
I push to Gerrit, check for build success, then Push to master.
Given that the bulk of failures are surely due to those updating
magic 'latest' contributions without any commit, why change the
rules for more diligent users?
Â Â Â Regards
Â Â Â Â Â Â Ed Willink

<https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient>
Virus-free. www.avast.com
<https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient>
<#m_-916945580506975019_DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
_______________________________________________
cross-project-issues-dev mailing list
To change your delivery options, retrieve your password, or
unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/cross-project-issues-dev
_______________________________________________
cross-project-issues-dev mailing list
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/cross-project-issues-dev

Frederic Gurr

2018-11-13 09:45:34 UTC

Permalink

Hi,

I have changed the submit type to "fast-forward only" for the
simrel/org.eclipse.simrel.build repo.

Regards,

Fred

Post by Ed Merks
Jonah,
+1 for fast-forward-only
In principle that could force one to rebase and then kick off another CI
Bot verification job, but that's not so likely to occur much in practice.
Cheers,
Ed

Post by Jonah Graham
Hi Ed,
If you want you can do a "Rebase" in the Gerrit UI before doing the
Submit.
Alternatively, and what I personally prefer is to set gerrit's merge
strategy to either Fast forward only, or rebase. The Eclipse platform
uses fast forward only. This prevents any merge commits and ensures
that the gerrit tests the same version as is pushed. The merge
strategy is a admin preference on the repo, so something that
webmaster/Fred would have to change IIRC.
HTH,
Jonah
~~~
Jonah Graham
Kichwa Coders Ltd.
www.kichwacoders.com <http://www.kichwacoders.com>
Hi
Ah! It never occurred to me that "Reply" meant "Commit" / "Push" /
"Merge" or even "Review". I saw only "Cherry Pick" / " Cancel"
which did not match my requirements. What a truly awful UI.
What if I don't want to "Merge" since it leads to double history
entries that on normal projects are difficult to disentangle? Ok,
SimRel commits are highly orthogonal. But Push gives a nice clean
history and forces a Rebase that just occasionally Merge can fail
to replicate. IMHO if a Merge is necessary the Gerrit Review
should iterate.
Regards
Ed Willink

Post by Ed Merks
Ed,
Normally you would use the Gerrit review link to finish the
https://git.eclipse.org/r/#/c/132049/
After the initial commit, I waited for the build to finish so
that CI Bot (one of the automatic reviewers) adds a +1. That
takes about 5 minutes. Then I used the Reply... (or the Review
+2 button, which will be there after the successful build) to
make it possible to "Submit" the changes to master, i.e., the
Submit button will be there once all the reviewers (CI Bot and
you) have voted the changes up to the necessary level.
After submitting, when I do a pull on the repo, my changes are
pulled and the repo is up-to-date (no longer one commit behind
master).
I believe a non-dilegent user could remove CI Bot from the review
to submit their changes even when those did not pass the
aggregation build but it appears to me that there really is no
good reason to allow direct push to master as a way to completely
bypass the CI Bot review.
Regards,
Ed

Post by Ed Willink
Hi
Sorry I must have missed something. My no doubt flawed
recollection was that we were assured (again) that direct push
would continue to be allowed.
I certainly use it every time since I see no other way to do it.
I push to Gerrit, check for build success, then Push to master.
Given that the bulk of failures are surely due to those updating
magic 'latest' contributions without any commit, why change the
rules for more diligent users?
Regards
Ed Willink