Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [theia-dev] New feature enabled for CI: Require branches to be up to date before merging
  • From: Vincent Fugnitto <vincent.fugnitto@xxxxxxxxxxxx>
  • Date: Mon, 27 Mar 2023 12:55:27 +0000
  • Accept-language: en-CA, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=ericsson.com; dmarc=pass action=none header.from=ericsson.com; dkim=pass header.d=ericsson.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=+gKX+Vn+tctNd1NyucRaiZ/ba/9VTtpyBgPmB6wxGN8=; b=c3Ywpt/rEfdtIAHkJ2Vz5l3hp922yOQMAA6GnuNU+KqZNj4GIOhV22v+p7L7oYKBG+mnHEiTT/gqr6HhRRofhSqCxPYZCmqR+VY+7vz7/Z7ZfbcYmmxi/dgq7pBxUob8/DFHbXDN6Smqz+X7eCJ2DEwauAfx+dWJW0McsvzbpOvDrLKIJ1Cc+lES1VGpVXWjnsJ/cpEXzKlfZ2VQenXVewT9cYZ8aA1ocNWDuwOvNL1Rd87qC3RJow7LjDSou8LxrllKABOz6Se3Y0T1X+zsLkwMcJaZd3SCa8ot5DA/TrCwbFJ3Cghy7i9WKoiFlptou/c48J7YuCeO4trsFLzo/A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KPtDV/9KCeqXpKh5c738Owysvz+q/FV2U7ZbrBGobWzCzVAUJr0/jgs48SbND0M5nfmBmptVkq03Hs4oGsBEXdCs1fZgE7GJ2BVcyT5aghKsEhwt0+ghwMITcNzbIE+EAWRLRE7JEJJFxWmnv8hN4OIzZcDxNDe6FU+IkJQQmxP0OmB8JU7fL5m2tMJRql6XXxDhFiP7zaw3Dr7yzrEsk5T4AGIRjbTN8ALa9e1b88EdxZu45hKBh02NfqZdDHiE7BcPeSPhWYbjiKxy07wrxLomiGRreG9lj1Qj87LJgjSUW0fPw/uJCfdxoL4xImW16x0jH6+4uiMQMj/7ONNdvg==
  • Delivered-to: theia-dev@xxxxxxxxxxx
  • List-archive: <https://www.eclipse.org/mailman/private/theia-dev/>
  • List-help: <mailto:theia-dev-request@eclipse.org?subject=help>
  • List-subscribe: <https://www.eclipse.org/mailman/listinfo/theia-dev>, <mailto:theia-dev-request@eclipse.org?subject=subscribe>
  • List-unsubscribe: <https://www.eclipse.org/mailman/options/theia-dev>, <mailto:theia-dev-request@eclipse.org?subject=unsubscribe>
  • Msip_labels:
  • Thread-index: AQHZXY7F2oE1JyYYQ0GvJQ3HifOszq8IbqqAgAABt+uABiillQ==
  • Thread-topic: [theia-dev] New feature enabled for CI: Require branches to be up to date before merging
Hi everyone,

I believe there are some significant drawbacks with the new "require branches to be up to date before merging" option during CI as it makes working in open-source (especially for non-committers) more difficult.

The main reasons are:
  • The process adds friction for non-committers when instead we want to encourage them to contribute
  • There does not seem to be the automatic "update" button during CI from forks unless explicity set by each contributor
  • Asking contributors to constantly rebase since master was updated adds friction, and it becomes increasingly difficult to get something merged from different time zones (ex: China) as master is a moving target
I think we may want to revisit the option, especially since we haven't really encountered an issue with an outdated pull-request often, and the drawbacks I've mentioned.
I can add a note in the dev meeting if needed.

Thanks!
Vince
From: theia-dev <theia-dev-bounces@xxxxxxxxxxx> on behalf of Marc Dumais via theia-dev <theia-dev@xxxxxxxxxxx>
Sent: March 23, 2023 11:00 AM
To: theia developer discussions <theia-dev@xxxxxxxxxxx>; Thomas Mäder <t.s.maeder@xxxxxxxxx>
Cc: Marc Dumais <marc.dumais@xxxxxxxxxxxx>
Subject: Re: [theia-dev] New feature enabled for CI: Require branches to be up to date before merging
 
Hi Thomas,

I had not thought about that, but it does make sense. In fact, I do see it happening on your PR about enabling context isolation, that originates from your fork, where presumably you have not granted permission for maintainers to edit. 

That, coupled with the need for one of our project committers (user with write access to our repo) to push the "Approve and run" CI button, post-branch-update, might make for a bit of back-and-forth between us and external (non-committer) contributors. 

One thing we could do to mitigate this is to update our PR template to encourage external contributors to grant us "allow edits from maintainers" on their PR? Maybe we can do this after the trial period of one month is over?

Regards,
Marc
From: theia-dev <theia-dev-bounces@xxxxxxxxxxx> on behalf of Thomas Mäder <t.s.maeder@xxxxxxxxx>
Sent: Thursday, March 23, 2023 10:36 AM
To: theia developer discussions <theia-dev@xxxxxxxxxxx>
Subject: Re: [theia-dev] New feature enabled for CI: Require branches to be up to date before merging
 
Very cool, thank you. 
I guess you need write access to the branch to be merged in order to see the "Update branch" button, right? Because it's your own branch or the contributor checked "allow edits from maintainers"?

/Thomas


Back to the top