Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jsp-dev] [External] : Benefits of using PRs for all changes
  • From: Ed Bratt <ed.bratt@xxxxxxxxxx>
  • Date: Fri, 5 Nov 2021 11:31:11 -0700
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.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=XhkKbwYOTcgMI1qT3sm0mUlLBujs1GWUP6OVseSf6l8=; b=R81M5X8dkoumrkkxC9NLKNH2vQBLWdYIRrURljUrBYVbWeuwRyCIKUhGrlq8mbvmcBb/3V9FuyDLqajYuaGuFiREe+PdkK6p8yH0xsbsG3FjDW9zWpnXvg7QVdbgR59Q2roaoge9j17CCi/0jDZL70WKoyHtvUCwpDcd8ORoBAIcHxOsAGkwV57WsQYX2f6uOaiQPJflFVp4BQqtZzmImsVFVw50vYqGBiW3YV7w5tg7SUAjtgcMM50FD7JasIL2b3b749pVsstr7DiSmaiimVMAq/0Df0cgiQPZa/OcSxg2YviYAwwg0F0ChSdsMBwecvfsz4LmI1Fv4lmLlL2pUw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=npjZmZLXqYV49T96i8Dss/Wy1yrA59XkmOBbTsnRdr0wFB9ZKZDnbSpOczkf2G6SyEsBIX9HU9aPgH9pXrsU+46C/Fb8sjtEqvOwOnSoYWXZGxnq1Su2C8I7Y8hGNCmi1tepinWBTDLfeVB9tf7Wv03H5u4VbUT01BjTUEuR5Atwvy+iXGscBANE68hbR2kwiNCa8WPvy2AURetNdXyxpv/84crXLk7baG30sHJPN1wFDwsN4qNX8OtLSQUxSZA1cDVYGjM5y46ZDOBW9jGc6BWRsARarfbq90YCgXufqsDb6Q4lpej7iUHmhna5BjJenysxBU+TJcOQ/V6cEq8d4Q==
  • Delivered-to: jsp-dev@xxxxxxxxxxx
  • List-archive: <https://www.eclipse.org/mailman/private/jsp-dev/>
  • List-help: <mailto:jsp-dev-request@eclipse.org?subject=help>
  • List-subscribe: <https://www.eclipse.org/mailman/listinfo/jsp-dev>, <mailto:jsp-dev-request@eclipse.org?subject=subscribe>
  • List-unsubscribe: <https://www.eclipse.org/mailman/options/jsp-dev>, <mailto:jsp-dev-request@eclipse.org?subject=unsubscribe>
  • User-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0

I appreciate your consideration of my comment. Please be assured I will be happy with whatever decision the group reaches and everything you've written below is valid -- nothing beats good commit messages and code comments. Whatever you arrive at, I'd recommend it be written even if just as notes -- in the CONTRIBUTING.md file.

Most of the rest is philosophy -- feel free to click delete now as you wish ....

I haven't tried using GitLab nor BitBucket. Both of these services have concepts similar to GitHub's Pull Request. GitLab calls it a Merge Request (or MR).

Having personally lived through several iterations on these repositories, I completely get your concern about moving the repositories from one VCS to another. I've lived with this from the ancient days of Teamware, a derivative of SCCS. Every time the repository system changes, some parts of the evolutionary chain are lost. We had to invest a fair amount of developer energy into each transition and it was never perfect.

All I can add is, while we are at GitHub, my preference is, if all things are otherwise equal, opt for the benefit of the lowly reader who comes to these repositories many weeks/months/years down the road and is trying to figure out -- how did some particular element of this repository get to this state -- and then possibly unravel that evolution. Specifications, in particular, can have a very long life-span. Something that seems innocuous to us today may, years from now be discovered as a source of some defect or another. All of it in spite of the best of our collective intentions. Sure, people will heavily rely on the blame logs.

In that regard, you are certainly correct -- the commit log record is paramount. The added benefit of collecting multiple commits together and integrating that with the issue tracking log is certainly an extension to Git. It is convenient not to force flatten merge requests (squash commits).

I think the GitLab Flow document does a good job describing the issues (or watch this video). Since GitHub is so ubiquitous, it's likely that any derivative hosting location with either a Git core, or some other newer and better technology, will have some kind of import scheme. That said, I can absolutely tell you, when we moved from Subversion to GitHub -- we had to develop some tools/scripts and even then, there was some loss of the record.

If we turn off PR approval requirements -- will calamity strike us if someone does a direct commit? Of course not. If we adopt some specifics in our policy but don't include hard controls, we have to accept that occasional 'quick fixes' will occur. If we have the policy, we can refer to it, if it's decided that any one of us is trending further away from what we've agreed to. At that point, the policy can be revisited for change -- or it can be used to remind the contributor what we have collectively recommended.

I don't want this to be harder than necessary and I recognize that, on occasion, a quickie change is needed. I can attest to the fact that even the GitHub web-UI now has better branch/PR/merge support and I've found that even those quickie changes can pretty easily be generated via an edit, branch-create, generate a PR followed immediately by clicking the merge button. So, even my excuses for short-cutting my own desires have been undermined.

OK -- I've gone on long enough. If you're still reading, thank you. Please adopt the policy and guidance you think is best for this group.

Thanks,

-- Ed

On 11/5/2021 9:39 AM, Mark Thomas wrote:
In the thread on relaxing the commit restrictions the view was expressed that changes via PR are always preferable to direct commits. I'd like to explore that.

I can see the benefits for use PRs to discuss substantive changes. Fore this project I'm broadly thinking of a substantive change as something that is going to require a change/addition to the TCK - something that changes the specification.

I don't see the benefit of using PR to - for example - fix a typo, correct a copyright date, fix an IDE warning (e.g. add a missing @Override annotation etc.).

Generally, I don't like PRs because they are GitHub specific. If we migrate away from GitHub - and this project has been through multiple version control systems in its history and I see no reason why its future will be different - then the information in the PR if not lost, will at least be detached from the source. I would much rather rely on good commit messages and code comments to pass on the 'why' of a change to the committers that follow.

The email archive of PR discussions is not easy to read contemporaneously. It is even harder to read for historic issues. I regularly find myself having to use the web interface to make sense of a series of comments on a PR I read on the mailing list.

I'm on the fence as to whether the benefits we get from using PRs is worth the risks. I think it is a very close call.

What to others see as the benefits (or risks) using PRs for everything?

Mark
_______________________________________________
jsp-dev mailing list
jsp-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/jsp-dev__;!!ACWV5N9M2RV99hQ!b4MaTBRlzLnCObAz0bmyN4JRHYWWr_B68qpCV0sZb9HWgj9yc9ZbMg3VhPfipY0$

Back to the top