Feedback for GitHub CEO

moneyball commented at 8:09 pm on April 18, 2019: contributor

Context from email from GitHub: “I run a program for our CEO Nat Friedman connecting him with community members in GitHub. It’s an hour long chat on Friday’s where you can talk about anything GitHub that’s on your mind. I was wondering if you all would like to join us. You’re welcome to bring other maintainers up to a max of about 7 people. Many groups bring a “top 10” list with them and we go through that. We also have some demos and mockups of stuff to show and then discuss. Tell us how to improve GitHub.”

This Issue is being created to collect ideas for the top 10 list. I would suggest ideas can be anything from annoying UX/reliability issues to feature requests to “crazy requests that might seem impossible.” Anything should be on the table.

MarcoFalke commented at 8:19 pm on April 18, 2019: member

Topologically sorting commits is highly requested already, but would be nice if you could take 5 seconds to tell them we are also interested in it. https://github.com/isaacs/github/issues/386

MarcoFalke added the label Brainstorming on Apr 18, 2019

MarcoFalke added the label Upstream on Apr 18, 2019

instagibbs commented at 8:24 pm on April 18, 2019: member

Stupid but: Backpointer from base repo to your own forked version, similar spot to “forked from”

MarcoFalke commented at 8:27 pm on April 18, 2019: member

@instagibbs This is already possible. Just click on Screenshot_2019-04-18 Feedback for GitHub CEO · Issue #15847 · bitcoin bitcoin

instagibbs commented at 8:33 pm on April 18, 2019: member

two clicks! ok I guess that’s fine :)

fanquake commented at 11:14 pm on April 18, 2019: member

annoying UX/reliability issues

Can we have a way to turn of the collapsing similar comments behaviour. i.e (#15564):

promag commented at 11:44 pm on April 18, 2019: member

Some nice to have features IMO:

regular contributor could set labels to own issues/pull requests
option to auto delete branch after merge
native WIP state
support –color-moved=dimmed_zebra
tooltip over acronyms :rage4:

msafi commented at 3:11 am on April 19, 2019: none

More intelligent code review experience. Like changed files being ordered based on their dependency graph, “go to definition” within code review, etc

JeremyRubin commented at 7:48 am on April 19, 2019: contributor

Verifying that a squashed branch is the same as a non squashed one would be awesome / better tools for managing PRs with multiple branches or alternatives.

Having a repository which can be pulled which gives you nicely structured local files for historical issues and pulls.

Tracking of ACK state per commit hash/utils to mark commits as reviewed.

Better linking from a commit to the historical pr that added it.

MarcoFalke commented at 11:55 am on April 19, 2019: member

@promag “native WIP state” is already supported: https://help.github.com/en/articles/about-pull-requests#draft-pull-requests

MarcoFalke commented at 11:59 am on April 19, 2019: member

Verifying that a squashed branch is the same as a non squashed one would be awesome

I think we should not rely on GitHub to do any integrity checks. Especially in light of the recent incident where a pull request’s branch was corrupted by GitHub: #15778 (comment)

promag commented at 12:02 pm on April 19, 2019: member

@MarcoFalke I see that as a “RFC/POC - don’t review code yet”, and also you can’t go back to draft. Setting to WIP should be possible if reviews cause the implementation to change. An alternative is to close/open in that case.

gwillen commented at 8:22 pm on April 19, 2019: contributor

Just for emphasis, here is the full set of IRC users (from a quick skim) who mentioned in the meeting this week that we should complain about not having topological/dependency order sorting of commits in pull requests: @instagibbs, me, wumpus, @sipa, @MarcoFalke, @jnewbery, @jonasschnelli. Plus @msafi in the thread above. At least four of these appear to be independent complaints and not just “me toos”, and I suspect more or less all of them are.

gwillen commented at 8:24 pm on April 19, 2019: contributor

Making draft/WIP (“not ready for merge”) state toggleable in both directions would be great – that’s what GitLab does and it works well.

I would also like to add (since I don’t see it here) gmax’s suggestion for moderation of issues/PRs: “a feature where you could set a repo so that newly created issues / PRs would only be visible to the submitter/project contribs (or at least logged in users) for 24 hours or whatever, in order to cut down abuse of github for trolling”

promag commented at 9:12 am on April 22, 2019: member

we should complain about not having topological/dependency order sorting of commits in pull requests @gwillen for the record me too.

jonatack commented at 10:54 am on April 22, 2019: member

Have an option to disable the annoying force-push warnings. Rebasing is too useful and necessary, among other reasons for encouraging hygienic commits, to be warned about constantly without an opt-out option.

promag commented at 11:06 am on April 22, 2019: member

@jonatack you mean in git push?

dagurval commented at 11:40 am on April 22, 2019: contributor

Full history for files. Currently a files history is missing from GitHub after a file has been moved.

gwillen commented at 12:39 pm on April 22, 2019: contributor

It would be nice to have a history of the versions a PR has gone through, with diff among arbitrary pairs of versions available, including across force-push.

I see that in fact you can get a before-and-after diff for any given force-push, by finding it in the “conversation” tab and clicking the words “force-pushed”, which is a good tip I didn’t know before. But having them all in one place (as GitLab does) would be really helpful for reviewing.

meshcollider commented at 3:53 am on April 27, 2019: contributor

A “bookmark this PR” system to keep track of interesting PRs I want to come back to would be great

MarcoFalke commented at 2:00 pm on April 29, 2019: member

It would be useful if there was a way to rerun all ci systems from a single button click (API call) on GitHub (Just a button that does effectively the same as close-open of a pull request). close-open-ing a pull request creates a lot of noise. E.g. #9384 (comment)

I am aware that travis and other ci systems offer a way to restart a failed build or schedule a new build, but that doesn’t update the ci configuration (travis.yml) to the latest version. So this reschedule feature is useless, if a re-run is needed because a bug was fixed in the yml or a silent merge conflict makes the yml no longer work with the latest merged pull request state.

Currently the only way to reliably reset the ci config to a clean state and rerun all ci systems (travis, appveyor, …) with one action is to close and open the pull request. Alternatively, a push or force push to the branch can be done, but usually only the pull request author can do this. And such pushes are generally discouraged because they invalidate previous review.

Finally, restarting a failed build from within travis will discard the log and thus makes it impossible to troubleshoot the issue later. Restarting the build from GitHub will preserve the old log and start a fresh build with a fresh log.

jonatack commented at 10:29 am on April 30, 2019: member

@jonatack you mean in git push?

Yes, the new-ish GH force-push warnings after each git push -f.

GitHub could also permit managing public options like this via a config file (e.g. .github_config, .github_config.yml, etc.) in the repo root under version control.

luke-jr commented at 4:55 am on May 2, 2019: member

It would be nice if contributors could re-open their own PRs. Right now, if the bot closes it for inactivity, we’re basically stuck…

MarcoFalke commented at 9:19 pm on May 3, 2019: member

There should be an option to disable the merge button.

Someone posted a comment while I clicked “Preview”. The new comment added enough space to move the merge button under my mouse pointer and I almost merged a pull request accidentally.

promag commented at 9:41 pm on May 3, 2019: member

@MarcoFalke doesn’t asks for confirmation?

kanzure commented at 7:45 pm on May 7, 2019: contributor

What about asking for sha-512 support in pullmerge requests?

moneyball commented at 8:23 pm on May 18, 2019: contributor

On #bitcoin-core-dev, @gmaxwell wrote: “I really wish there were more ability to turn off parts of the github page that projects don’t use. The whole setup effectively tries to get you to use github as the projects’ website, but doesn’t make it sufficiently customizable. Displaying contributors on the front in alphabetical order, and the only way to control it is to make people hidden is another example.”

luke-jr commented at 7:40 pm on May 30, 2019: member

Making draft/WIP (“not ready for merge”) state toggleable in both directions would be great

How would this differ from a WIP label (if PR submitters can set their own labels)?

luke-jr commented at 7:42 pm on May 30, 2019: member

GitHub has a nice review interface, but it’s of limited usability because it’s trusting GitHub to show the diff faithfully. (This is why many of my ACKs don’t have commit hashes.)

I’m not sure how, but it would be nice to use the review interface with a locally-sourced diff. Uploading a diff probably won’t help, as it suffers from the same problem “server doesn’t store your keys” webwallets do. Maybe would need an open source local (Qt, ideally) application.

MarcoFalke commented at 10:34 pm on May 30, 2019: member

I think there is nothing that can remove the need to sign the review comment. And signing the review comment only makes sense after you did a review of a locally-sourced diff.

MarcoFalke commented at 10:37 pm on May 30, 2019: member

Generally my local reviews are signed. Example: #15988 (comment)

luke-jr commented at 2:54 am on May 31, 2019: member

I think there is nothing that can remove the need to sign the review comment. And signing the review comment only makes sense after you did a review of a locally-sourced diff.

Sure, my point is to bring the visual review interface to local git. Maybe it can even be a git plugin.

MarcoFalke commented at 11:21 am on June 5, 2019: member

@Sjors mentioned that there should be a way for GitHub to not hide comments.

Maybe a global setting or at least a button to unhide all?

kanzure commented at 12:45 pm on June 5, 2019: contributor

One thing that came up in our Amsterdam discussion is adding a checkbox to code comments on PRs, where the author of the comment can choose to mark the comment as a “nit” that doesn’t blast out email notifications unless the PR author (or the watchers) have enabled nit notifications. Similarly, the PR review interface could also filter or hide nits if the user checks a box or hits a button.

kanzure commented at 12:50 pm on June 5, 2019: contributor

Also from the group discussion: Copying a hash from github user interface so we can write ACK <commitid> more easily. Some easy way to copy the commit hash without doing surgery.

jonatack commented at 12:57 pm on June 5, 2019: member

My understanding has been that it’s best to not copy the commit hash from a trusted third party (e.g. by right-clicking the commit in GitHub and copy-pasting) in favor of using the commit hash from review in the local dev environment.

jonatack commented at 1:09 pm on June 5, 2019: member

It also helps ensure ACKing of the exact changes.

kanzure commented at 8:17 am on June 6, 2019: contributor

Organizations should be able to enforce their members do not have SMS recovery enabled. Right now it seems they can see on a page whether the user has it enabled, but not any kind of enforcement.

kanzure commented at 9:37 am on June 6, 2019: contributor

From wumpus: a permission category for users that can only add, edit issues and add, edit tags on issues, but not push to any branch. Protected branches doesn’t really work because you have to remember to protect branches.

moneyball commented at 9:38 am on June 6, 2019: contributor

From discussion today: have more fine grained control about what requires commit access and does not. For example, there is a desire to allow people to add/remote tags without having commit access.

moneyball commented at 12:27 pm on June 7, 2019: contributor

I discussed this with @MarcoFalke, @ryanofsky, @laanwj, @fanquake, @achow101, and @theuni at lunch today. Here is a draft plan for the lunch.

Goal: Persuade GitHub CEO that the Core project is important enough and unique enough to warrant investing design and dev time on understanding how the project pushes the limits of the GitHub design. Get a commitment for a follow up with a GitHub designer.

Plan:

Provide overview of Bitcoin Core project
Highlight specific aspects of the project that stress GitHub UX
Show examples / give demos

The aspects that will be highlighted:

Better granularity of access control (eg don’t require commit access to modify labels)
UI clutter / notifications noise (nits, downstream projects, unbatched comments)
Merging PRs (structured ACK data) @theuni can you add other details I missed, particularly adding more detail about merging PRs?

moneyball commented at 1:56 pm on June 9, 2019: contributor

IIRC there were past complaints about GitHub sorting of PRs. I think the complaint is that if you sort by “least recently updated” it isn’t reflective due to downstream projects (and other spurious activity?) updating PRs in a way that we would not consider them being “active.”

Is this accurate? If so, what would be our request to GitHub?

Empact commented at 2:11 pm on June 9, 2019: member

Similar to @promag’s point, would be helpful if regular contributors could add tags and request review on their own PRs.

moneyball commented at 2:55 pm on June 9, 2019: contributor

I spoke with a few folks offline, and my suspicion above about downstream project activity isn’t correct. However, drahtbot is the biggest culprit in which its comments make a PR “active” even if it is not active.

moneyball closed this on Jun 9, 2019

moneyball commented at 2:56 pm on June 9, 2019: contributor

Uh…that was an accident to close it. I fear I’ll now be unable to re-open it? I guess this is another request for the GitHub CEO :)

meshcollider commented at 2:57 pm on June 9, 2019: contributor

Reopened for you @moneyball

meshcollider reopened this on Jun 9, 2019

fanquake pinned this on Jun 10, 2019

fanquake commented at 9:09 pm on June 10, 2019: member

Added this to the pinned issues list for a bit more exposure before the lunch.

moneyball commented at 7:22 pm on June 11, 2019: contributor

@gmaxwell had posted the following to #bitcoin-core-dev many months ago. This is related to the noise issue we want to raise with GitHub.

“ Gregory Maxwell breaking sort by recently updated has probably reduced the amount of review I do by 80%, FWIW.”

Also @jnewbery mentioned this in the same session:

“ John Newbery MarcoFalke: sort-by-most-recent-change used to be the only thing I used, but it’s mostly not very helpful now”

jnewbery commented at 8:01 pm on June 11, 2019: member

sort-by-most-recent-change used to be the only thing I used, but it’s mostly not very helpful now

I think this was @MarcoFalke

MarcoFalke commented at 8:06 pm on June 11, 2019: member

I don’t use sort-by-most-recent-change, but I will try to rework DrahtBot to not change the comment any more.

moneyball commented at 8:34 pm on June 11, 2019: contributor

Here are the references FWIW: http://www.erisian.com.au/bitcoin-core-dev/log-2018-11-01.html#l-272 http://www.erisian.com.au/bitcoin-core-dev/log-2018-11-01.html#l-319

jnewbery commented at 8:42 pm on June 11, 2019: member

@moneyball - the thing that gmaxwell and I were complaining about was that when drahtbot posts in PRs, it marks them as updated, and sorting PRs by most recently changed shows the PRs that drahtbot has commented on at the top. That’s not a github issue - it’s an issue with the way drahtbot comments on issues.

MarcoFalke commented at 8:56 pm on June 11, 2019: member

It could be interpreted as a GitHub issue, since any change in any comment in a pull request will mark the whole pull as “updated”

sipa commented at 9:23 pm on June 11, 2019: member

Yeah, a separate “sort by last modified PR tree” or so option would be great.

JeremyRubin commented at 9:54 pm on June 11, 2019: contributor

I’d love for there to be some sort of better notification system for email-only participants to get comment edits.

MarcoFalke commented at 10:01 am on June 12, 2019: member

Actually, I am not going to modify DrahtBot to store the metadata it produces externally. This won’t solve the underlying issue with GitHub and if we keep more and more data outside of GitHub (e.g. https://bitcoinacks.com/ and https://drahtbot.space/), we might as well get rid of GitHub completely and host a modified version of GitLab or so.

See: Moving to self-hosted issue and patch management #13411

MarcoFalke commented at 5:59 pm on June 17, 2019: member

Another suggestion is that the current permission system is overly restricted. See https://help.github.com/en/articles/repository-permission-levels-for-an-organization#repository-access-for-each-permission-level for a table of all permissions, which is separated into a few categories from (read-only to admin). However, often we want to give very specific permissions. Let’s say a mix between Triage and Write for e.g. DrahtBot.

MarcoFalke commented at 4:05 pm on June 20, 2019: member

The sort-by-tree-changed is implemented on the third party site: https://bitcoinacks.com/?desc=1&flt0_mergeable_contains=Mergeable&sort=11

moneyball commented at 0:43 am on June 22, 2019: contributor

Today, @sipa, @theuni, @TheBlueMatt, and I met with a team at GitHub including their CEO, a couple of product guys, their lead designer, and their head of developer relations. Beforehand, we put together this prep doc (https://docs.google.com/document/d/1g5VWiIoAvngNhy0wTibA7t8tkoCTpeyOtVZcz5ao6jY/edit?usp=sharing) summarizing the feedback from this Issue, and condensed things down to something we could share in ~30 minutes.

The meeting was a success and can be summarized:

We communicated the feedback from the prep doc to them. They (particularly their CEO) seemed to fully understand and appreciate the feedback, particularly the trust minimization philosophy of the project.
We have a direct line of communication now with the folks we met in case we need to follow up on a design issue, security issue, etc.
They demo’d some upcoming work which looked like it could be applicable to the project. Cory plans to engage them on that. They asked us to respect confidentiality until it is announced.
We asked about their Sponsors program, in which they match sponsorship up to $5k per developer, and whether they’d considered accepting bitcoin. I plan to follow up with them on that.

fanquake commented at 1:20 pm on June 22, 2019: member

@moneyball Nice work. Are next steps essentially just wait and see what happens out of GitHub?

I’ll unpin this issue for now.

fanquake unpinned this on Jun 22, 2019

moneyball commented at 3:00 pm on June 22, 2019: contributor

Next steps:

Cory to engage them on the new changes to provide feedback and evaluate for Core’s usage
Once announced, other contributors can evaluate and the project can determine whether or not to use
We didn’t have any specific asks to change things, just painted broad strokes so they understood our needs (eg more granular permission etc.). So, we can wait and see, or, if we have specific (high priority asks, we can follow up directly with them

On Sat, Jun 22, 2019 at 6:22 AM Michael Ford notifications@github.com wrote:

@moneyball https://github.com/moneyball Nice work. Are next steps essentially just wait and see what happens out of GitHub?

I’ll unpin this issue for now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/issues/15847?email_source=notifications&email_token=AACPUA6U4YSVQRM7KTLXBI3P3YRS5A5CNFSM4HG77LIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYKJJ3Q#issuecomment-504665326, or mute the thread https://github.com/notifications/unsubscribe-auth/AACPUA7MTGS6QRL3VRBZNFDP3YRS5ANCNFSM4HG77LIA .

MarcoFalke commented at 3:10 pm on July 3, 2019: member

Is anyone else seeing a non-responsive GitHub UI? E.g. when switching to the “commits” tab or “files” tab it will often hang for me and I need to reload the whole page.

hebasto commented at 3:51 pm on July 3, 2019: member

Is anyone else seeing a non-responsive GitHub UI? E.g. when switching to the “commits” tab or “files” tab it will often hang for me and I need to reload the whole page.

I have observed such behavior sometimes.

moneyball commented at 10:17 pm on August 30, 2019: contributor

Hello, I received an email today from GitHub asking if anyone on this project would like to serve in the role described below. If interested, let me know and I’m happy to connect you with Bryan.

“Hi Steve -

I really appreciate the time you all spent with us at GitHub in June. I wanted to offer this invite to you or someone from your team to join our Open Source Advisory Board this year. Here’s the full invite text below:

In conjunction with this year’s GitHub Universe (https://githubuniverse.com/), we are hosting an Open Source Advisory Board, to help us understand how we can better serve the Open Source Community, and what you would like to see from GitHub in the coming year. We would like to invite you, or someone else from your project, to participate in this Advisory Board.

Our aim with this board, and this invitation is to seek honest feedback from across the Open Source community (whether you use GitHub or not) about what GitHub can do to best support it. We would earnestly like to hear your opinions.

This year’s Open Source Advisory Panel will be held on the afternoon of Tuesday 12 November, in San Francisco, California. Just let us know. Of course, you will also receive complimentary tickets to attend GitHub Universe on 13 and 14 November.

Please RSVP by replying to this email if you are interested in participating. We are looking forward to have you join us!

Thanks, ~ Bryan”

fanquake commented at 1:15 pm on August 31, 2019: member

If interested, let me know and I’m happy to connect you with Bryan.

I’ve volunteered to do this, so should hopefully be heading over to SF in November.

achow101 commented at 6:03 pm on September 17, 2019: member

Suggestion:

When people respond to conversations in a review, make it easier for others to actually find the original conversation where they can respond to it. Otherwise, it requires scrolling through potentially hundreds of comments, comments that may be resolved and now hidden, and needing to have to click every “show X hidden comments” just to figure out where the hell a conversation is in order to be able to respond to it.

It would be so much better to either have a link that lets you jump to that convo, or even better, move the convo down to the review with the latest comment so that the review doesn’t just have a blob of text with absolutely 0 context.

ryanofsky commented at 2:35 pm on September 19, 2019: member

Please GitHub lord and savior grant me a “Load All…” link for truncated diff and issue pages. It’s really unpleasant when you want to do something like review all the changes in a PR, or Ctrl-F in a page but can’t find things because of content hidden behind “Load More…” links.

JeremyRubin commented at 11:49 pm on September 24, 2019: contributor

Some sort of CTags support while browsing on the web would be really nice.

achow101 commented at 6:10 pm on November 7, 2019: member

Occasionally people add various non-github contributors’ emails to their accounts and GitHub lists their account as the owner of the commits. We took care of most of that a few years ago by asking them to move those emails to @non-github-bitcoin, but it turns out a few emails were missed and other people have claimed them. It would be nice if GitHub would have an easier way to deal with this, or just not list people’s accounts for unverified emails.

jkczyz commented at 0:09 am on November 9, 2019: contributor

Pull requests could use attention sets, indicating which users need to respond or take some action. They would be constructed from the PR’s author and reviewers (or those who’ve commented) and updated automatically as comments are added / resolved. Manual updating the attention sets could be supported as well.

The workflow would be something like the following:

When a reviewer starts or responds to a comment thread, the author is added to the thread’s attention set and the reviewer is removed.
When an author responds to a comment thread, the other thread participants (or maybe just the last reviewer?) are added to the thread’s attention set and the author is removed.
When someone is mentioned in a comment thread, they are added to the thread’s attention set.
When a comment thread is resolved, its attention set is emptied.

Note: By comment thread I mean the comments on a line or block of code. Ideally, top-level comments would also be threaded / resolvable and would work the same way.

Each PR would have an overall attention set consisting of the union of all comment threads’ attention sets. Then the overall attention set would indicate who needs to take action on the PR. All PRs that a user needs to take action on could be surfaced as a dashboard or by using a search qualifier.

This could be implemented as an improvement on the “Reviewers” and “Assignees” PR features.

MarcoFalke commented at 3:58 pm on April 26, 2020: member

Not sure how much GitHub is addressing our feedback or paying attention, but the results right now seem limited. Closing for now, we can reopen this when there is a place to track issues and see how/when/and if they are going to be addressed.

MarcoFalke closed this on Apr 26, 2020

fanquake commented at 9:34 pm on April 26, 2020: member

when there is a place to track issues and see how/when/and if they are going to be addressed.

We do have a place to track issues (in the open source maintainers group), and currently have multiple issues open there.

DrahtBot locked this on Feb 15, 2022

Feedback for GitHub CEO #15847