From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1EFF521B02822 for ; Mon, 22 Oct 2018 03:14:48 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2982281DF1; Mon, 22 Oct 2018 10:14:48 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-1.rdu2.redhat.com [10.10.121.1]) by smtp.corp.redhat.com (Postfix) with ESMTP id E51FB1001F40; Mon, 22 Oct 2018 10:14:46 +0000 (UTC) To: Jeremiah Cox , stephano Cc: "edk2-devel@lists.01.org" References: From: Laszlo Ersek Message-ID: Date: Mon, 22 Oct 2018 12:14:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 22 Oct 2018 10:14:48 +0000 (UTC) Subject: Re: TianoCore Community Meeting Minutes X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Oct 2018 10:14:49 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hello Jeremiah, On 10/19/18 18:09, Jeremiah Cox wrote: > I'd like to better understand assertions of GitHub "lock in". GitHub > provides a comprehensive REST API that you can easily drive via Python > ("pip install PyGithub"). > I recall one assertion that GitHub holds PR comments hostage, but it > is trivial to dump out all Comments on all PRs in a repo. > > > g = Github( ) > repo = g.get_repo("chipsec/chipsec") > pulls = repo.get_pulls(sort='created', base='master') > for pr in pulls: > for comment in pr.get_review_comments(): > print(pr.number) > print(comment.body) > ... > > Can folks help me better understand examples of GitHub "lock in"? I > think it would be trivial to author a daemon that listens for PRs and > comments, and forward those to a mailing list, if that is preferred > for archival. > With respect to web-based, we believe it facilitates convenient, > multi-platform access, and GitHub's REST APIs can be leveraged to > provide a command-line driven experience. I believe Sean's point was > that GitHub provides a 1-stop shop for code reviews and issue tracking > such that there is no need to spend TianoCore resources set up, > maintain, and update separate Bugzilla and Gerrit services, VMs, OSs, > & machines. The REST APIs could be used to maintain a mailing list or > other mirror. I am not asserting that GitHub provides the best code > review and issue tracker experience, our team finds Azure Dev Ops is > superior, but GitHub is sufficient for most use cases while handling > the infrastructure so that we can focus on getting things done. > > I think it would be helpful to construct a PROs/CONs table for each of > the proposed end-to-end solution (source control, code review, gates, > CI/CD, issue tracking, testing, ...). Perhaps this table is on a wiki > to enable faster iteration than emails on the DL. > We think of "Pull Request" as a contribution & review process akin to > GitHub, GitLab, or Azure DevOps. Feedback, responses, automated > check-in gates, and signoff can be handled within each PR ticket. > Policies can be assigned, automated tests that must pass, enforcement > of community wide rules (# of reviewers, reviewer list, all comments > "resolved"). Tests could start as simple as successful build and over > time could grow to a more comprehensive set of boot, functional, > static analysis, ... > By leveraging a popular hosting provider's built-in workflow we enjoy > significant efficiencies. Its fully documented and known to most. New > capabilities are being added daily. No TianoCore resources would be > required to manage systems, OS patches, network issues, etc and can > instead be focused on TianoCore code, building tests, and driving > process improvements. I don't have anything against GitHub specifically. I also don't have any "favorite" WebUI (beyond Bugzilla, of course :) ). I'm concerned about a number of general anti-patterns that might not be obvious, and might not be easy to prevent with any given modern web application. Perhaps I can word this as a personal wish list: * Full email audit trail (all changes mailed out, even own ones), about patch submission, PRs, patch review comments, and bug tracker actions alike. One should be able to establish the full history of a bug or a pull request just from the emails. (This would be for human readers, not for machines.) Review comments made on the web, in reference to specific parts of a patch, should preserve that context in email form. The messages generated for a given bug or PR should form a thread on the mailing list. (Proper use of In-Reply-To and References headers.) The generated emails should be readable without HTML support (i.e. as plain text). As a counter-example for this, I'll mention JIRA -- JIRA's emails are practically unreadable without HTML rendering support in the MUA. If a WebUI can offer such an email gateway as a feature, that's awesome. It is fine if the gateway is one-way only (web to email). The service should not be an external one however, in order to minimize email lossage when the WebUI works but the gateway goes offline for some reason. The gateway should be tightly integrated. * The comment editing interface on the WebUI should support, in addition to the default rich editing interface, a plaintext mode that uses monospace font, interprets no markdown sequences, and preserves the exact layout of the plaintext comment (including indentation and line wrapping). Counter-examples abound. Perhaps somewhat unexpectedly, I'll call out Launchpad. Among other things: (a) Launchpad truncates comments it thinks are "too long", in the full bug display -- and then readers have to open the bug comment separately, to see it in its full length; (b) Launchpad inserts invisible breaks into words it thinks are "too long" -- and then, if someone double clicks a 40-nibble commit hash in the middle, they will not see it wholly selected; only a middle section will be selected between the hidden word breaks. The invisible word breaks will also allow commit hashes to be wrapped. * A similar "stick to basics" criterion exists for commit message authoring, and display. Commit messages are to be authored with monospace font in mind, and 74 character line length. Commit messages should not be reflowed or rewrapped automatically on display. BZ numbers and commit hash references may be highlighted and turned into actual links, but they should not be shortened or otherwise reformatted. The original formatting of a commit message is not a mindless process, and any WebUI should respect that, and preserve the original author's work, for display too. * Old versions of patch series (pull requests) and their associated review comments must remain publicly available forever. No historical review comments must ever turn into dangling references on the WebUI, nor must old (rejected) pull requests (topic branches) ever be possible to delete (or to overwrite by force-pushing), even for the original author. * No "squash on merge". Structuring a patch series is a first class development activity, and the git history is a first class aspect of the codebase. * More generally, admins should have fine-grained control to disable unwanted WebUI features. * "Edit by reviewer on merge" should be restricted to minimally invasive changes, and participants must agree on such actions up-front. * I don't know how a WebUI can support "rebase by maintainer". On one hand, such a feature would have to be used very carefully (see the previous point); on the other hand, adding Reviewed-by and similar tags without it is not possible for the maintainer. * Support for displaying and perhaps editing "git notes" (see git-notes(1)) would be a plus. They permit developers to attach patch-level changelogs (incremental change descriptions) to individual patches, such that these changelogs never make it to the final git history. Such notes are useful for repeated reviews, but not really useful for the long-term git history (which does not capture the discussion context / previous versions of the pull requests anyway). Thanks! Laszlo