On 11/26/18 22:43, Jeremiah Cox via edk2-devel wrote:
> Feedback on GitHub as follows…
> 
> 
>> 1. No Lock-In - What automated data export is available?
>> We want to be able to leave and take all our data with us. "Data" here 
>> includes: review comments, pull requests / patches (including metadata), 
>> old (rejected) pull requests and metadata, issue tracker entries and 
>> comments (if issue tracker included). This archiving should be 
>> automated, not something we do by hand.
> 
> Untested, but might these all be easily satisfied by subscribing a mailing 
> list to GitHub notifications?  
> https://help.github.com/articles/about-notifications/#watching-notifications 
> https://help.github.com/articles/about-email-notifications/ 

No, they are insufficient.

Following the last link above ("about-email-notifications"), one finds
several other links; and one of those is:

https://help.github.com/articles/about-notifications/

This article says,

    GitHub sends participating notifications when you're directly
    involved in activities or conversations within a repository or a
    team you're a member of. You'll receive a notification when:

    [...]

    - You open, comment on, or close an issue or pull request.

    [...]

This is demonstrably false. I'm a member of the TianoCore organization,
I have commented on, and closed (rejected):

  https://github.com/tianocore/edk2/pull/133

and I *never* received an email notification about my *own* comment /
action. I only received the initial email, about the pull request being
opened (attached for reference).

* Another pull request, demonstrating the same issue (original email
also attached):

  https://github.com/tianocore/edk2/pull/127

* And here's the same problem, just in a different situation: someone
made a comment on a commit, using the github WebUI:

https://github.com/tianocore/edk2/commit/253d81c71f67e1ab218450b87370abd3bf01d571#commitcomment-27830037

I responded there. I received an email -- attached -- only about that
other person's initial comment, and never received an email about my own.

So, no. I'm already subscribed to github notofications, and their
coverage is insufficient.

> 
> Alternatively, the GitHub REST API appear to offer full export capability of 
> all information & metadata:
>    https://developer.github.com/v3/git/commits/#get-a-commit 
>    https://developer.github.com/v3/pulls/#list-pull-requests
>    
> https://developer.github.com/v3/pulls/comments/#list-comments-on-a-pull-request
>    https://developer.github.com/v3/issues/events/#list-events-for-a-repository
>    https://developer.github.com/v3/issues/comments/#list-comments-on-an-issue 
>    https://developer.github.com/v3/activity/events/#list-repository-events 
>    
> https://developer.github.com/v3/reactions/#list-reactions-for-a-pull-request-review-comment
>  
>    * the above allows you to export all of the thumbs up/down, smileys, 
> hearts ... that users have given to pull request & issue comments  :)

This is again insufficient. We shouldn't have to cobble together our own
archival soluion from low-level APIs.

GitHub has extremely good availability. I doubt that any hack we could
come up with (and that we'd have to run ourselves, elsewhere), could
muster the same service level. This means that sooner or later our
mirroring hack would go down, while GitHub would stay up, and then we'd
start losing updates to our "mirror".

The offline & full coverage audit trail has to be generated by a core
part of the service.

[...]

>> 3. Flexible Workflow - Can we use email patches / email review as well 
>> as pull requests / web UI review?**
>>   3a. Can we can attach review comments to specific code *and* commit 
>> message locations?
>>   3b. Are the comments faithfully translated to notification emails 
>> (including the locations in code the comment is addressing)?
>>   3c. Are old topic branches (rejected or updated pull requests) 
>> available even after being rejected? (i.e. are they ever deleted?)
>>   3d. Is plain text supported in code review comments?
>> **To be clear, it is acceptable if the system handles only pull requests 
>> and a web UI. We do require, however, a *read-only* email notification 
>> system that thoroughly documents our process.
> 
> We propose that all review & issue tracking are through GitHub web, REST, or 
> Graph APIs.  Email becomes read-only for notification and archival only.
> 3A:  Yes.
> 3B:  From our testing this appears to be yes.
> 3C:  GitHub can be configured to keep rejected and updated pull requests.  
> 3D:  Both plain text and markdown work

This sounds good, but can you please clarify 3C?

In particular, what does an "updated pull request" mean?

Here's the specific workflow I care about.

* Alice implements a new feature for edk2 and opens a pull request. The
pull request refers to her branch that is called "alices-grand-feature",
with the branch HEAD at commit FOO.

* Brenda reviews the commits on that branch, and makes some comments on
various patches:
- she attaches some requests for clarification to specific lines of some
commit messages (i.e., not code),
- she also attaches some suggestions to specific code lines.

* A few days later Alice comes back with the reworked patch set. In her
own repository, she rebased the "alices-grand-feature" branch, and now
the HEAD points at commit BAR. In Alice's own repository, the original
HEAD commit FOO is now lost (no branch references it). So are all the
commits on the original (v1) topic branch that used to lead up to FOO.

* Brenda gets an email notification that Alice refreshed her pull
request, with the remote topic branch -- which is offered under the same
name "alices-grand-feature" -- now pointing at commit BAR. Brenda has
since forgotten some of the comments she had made under v1, and now she
scrolls up to see the original v1 commits (culminating in FOO) from
Alice, and her own v1 comments.

GitHub supports this -- i.e., the complete git history leading up to
now-unreferenced commit FOO will be preserved for eternity, together
with all location-sensitive comments made for it -- because: [[please
fill in]].

> Some additional questions we feel are important:
> 
> 
> *  Does the workflow facilitate automated validation & PR-Gates?  
> GitHub: Yes
> Phabricator:  https://secure.phabricator.com/T9456 : “Writing lots of 
> integrations for third-party software is broadly something the upstream is 
> not well equipped for.”
> Travis-CI further declined support for Phabricator: 
> https://github.com/travis-ci/travis-ci/issues/2143#issuecomment-124150608 “we 
> have no immediate plans to add this feature”
> 
> 
> *  Does workflow allow easy contribution process?  
> GitHub:  Yes, well-known and well-documented
> 
> 
> * Does it have comprehensive documentation?
> GitHub:  Yes
> 
> 
> *  Does it have a comprehensive programmatic API that enables extensibility, 
> with numerous online examples?
> GitHub:  Yes 
> 
> 
> *  Does workflow facilitate different server-enforced policies for different 
> branches?
> GitHub:  Yes 

Right, those are good and important features.

Thanks,
Laszlo
--- Begin Message ---
When the CLA was created from the Apache ICLA, the option for a CCLA was 
removed. When this was done the 'or' was lost in the text. This puts it back in 
so that you represent that you have received permission __or__ your employer 
has waived such rights.
You can view, comment on, or merge this pull request online at:

  https://github.com/tianocore/edk2/pull/133

-- Commit Summary --

  * Adding missing 'or'

-- File Changes --

    M Contributions.txt (2)

-- Patch Links --

https://github.com/tianocore/edk2/pull/133.patch
https://github.com/tianocore/edk2/pull/133.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/tianocore/edk2/pull/133

--- End Message ---
--- Begin Message ---
bin files for different architectures are now located in SDK version specific 
sub folder
You can view, comment on, or merge this pull request online at:

  https://github.com/tianocore/edk2/pull/127

-- Commit Summary --

  * Fix for Microsoft Visual Studio 2017

-- File Changes --

    M BaseTools/Conf/tools_def.template (2)

-- Patch Links --

https://github.com/tianocore/edk2/pull/127.patch
https://github.com/tianocore/edk2/pull/127.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/tianocore/edk2/pull/127

--- End Message ---
--- Begin Message ---
Hi, I'd learned that currently only 1024*768 resolution is supported for win7, 
however,  i'd like to know why and is there any plan for further support for 
other resolutions on win7 system.

-- 
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/tianocore/edk2/commit/253d81c71f67e1ab218450b87370abd3bf01d571#commitcomment-27830037

--- End Message ---
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to