Graphite Community

Interesting thought! Yeah, like you said this does exist on the PR view under the `show stack` tab - if you select the tab and _then_ copy link to the PR to share with people, it will direct them to the stack tab pre-opened if that helps!

That should only be happening if there is a pending mutation, do you have a repro of this happening for just a page load?

yep, every time I go from review queue -&gt; single diff

<@U047KHH8JH3> is putting the finishing touches on this :slightly_smiling_face:

Thanks for the feedback and I’ll let you know once it is launched!

if you’re interested in Graphite notifications in Slack, join us in <#C04L3N3H34J|notifications-beta>!

<@U04AP0M9AES> I'm already in the beta, but my feature request isn't about notifications, it is about redirecting any graphite PR link to its corresponding GitHub link

ah, got it — is there any particular reason that you want to be redirected to github?

I am usually more comfortable with reviewing PRs on Github (matter of habit, I guess) and Graphite uses graphite.dev hyperlinks in the CLI and in the notifications, which makes me unable to directly go to Github (I need to manually go to the PR)

makes sense! we’d love to hear about what we can do to support your review workflow with Graphite :slightly_smiling_face:

I think I was in the same boat as Youssef for awhile (I’ve been trying to use the Graphite dashboard more this week for reviews to get used to it)

I took a few minutes to compare the GitHub Review page and the Graphite Review page, and I think for me, the discomfort mainly boils down to familiarity and colors?

The Graphite review page feels (emphasis on personal feelings) more busy because the boxes for each file stands out more,  the general info has lots of boxes, and there’s the additional description box. So when you land on the page it feels more overwhelming? (Though part of this is also I think being used to GitHub’s design and needing time to get used to the Graphite design).


TL;DR: For me, the discomfort is a me thing, not a Graphite thing. Though I think I would like the design to move towards a smoother, less busy style (obligatory caveat; i’m not a real designer i have no design skills :joy: )

thanks for the feedback! we’re actively working on revamping the review experience and this is really helpful (cc <@U02DCQ1C0G6>, our design team :wink:)

also, could you expand on what we can do to make the colors more familiar/comfortable?

Screenshot 2023-02-03 at 11.18.18 AM.png

Screenshot 2023-02-03 at 11.20.02 AM.png

Screenshot 2023-02-03 at 11.18.07 AM.png

Hmmmmm I think it’s ok for the colors to be different? My hypothesis is that there’s more cognitive load because all of the boxes standout, so there’s not one place that my eyes are drawn to (too many competing pieces of information). While GitHub’s Files changed section, for example, basically everything blends into the background except the lines that were changed.

The diff itself is also slightly simpler? For example, there’s extra highlights in the Graphite review page than the GitHub review page (see screenshots). In the examples I attached, the extra green bits on top of the whole chunk seem extraneous? I think GitHub usually only includes those if the line is basically the same except to highlight a one word change.

TL;DR: there’s lots of things competing for my attention on the graphite review page which makes it harder to focus on the code itself (at least I think this is what it is)

that’s really useful context, thank you! cc <@U047KHH8JH3>, who’s currently thinking about ways to uplevel diff presentation!

I agree on Leslie's feedback, I am able to focus more on the code and diff in Github, also, the vibrant colors on Github helps in grabbing attention to diffs

In addition to that, Graphite isn't very smooth on big PRs, the scrolling is a bit laggy and sluggish, so it is a big discouraging to use.

One more, the description section is reaaaaaally wide, it is much easier to read PR descriptions on Github because the description section is centered and is smaller. Also, the gray text makes it really hard to focus on reading descriptions.

Adding to that is that my eyes are more used to Github (naturally), so I also find myself going to Github for reviewing PRs.

we hear you! passing all of this along :slightly_smiling_face:

cc: <@U02DCQ1C0G6> who was thinking about the side panel

definitely! <@U02DCQ1C0G6> i'm about to come to your desk to bother you for some quick designs :stuck_out_tongue:

Screenshot 2023-02-02 at 15.11.31.png

ha! is this just appeared??? Or I didn’t see it before?

Screenshot 2023-02-02 at 3.14.40 PM.png

Ah good catch! This is definitely a little confusing - cc <@U02DCQ1C0G6> this could be a quick design change similar to selecting reviewers for a PR!

Cc <@U047KHH8JH3> this should now be supported with your change right?

Thanks for the feedback <@U04H4LB3K9S>! This should be added now, LMK if you see any issues.

We’re working on a bunch of new features for merge in our next cycle, thanks for the feedback &amp; stay tuned for some updates soon!

Screenshot 2023-02-07 at 11.35.23 AM.png

to clarify you're selecting multiple images/screenshots and dragging them into the description at once?

Ooh ok thanks for the suggestion! (cc <@U03Q89DJPL4> for image uploads)

CleanShot 2023-02-07 at 15.42.07@2x.png

I also run into this alll the time. Maybe y’all could sacrifice the indentation of the merged PRs so we get better indentation of the unmerged ones.

ideas:
• put the merged ones into a collapsible `&lt;details&gt;` block ?
• add a level of indentation per sub tree, not per PR ?
• like
• this
    ◦ other
    ◦ branch
• first
• one
• with
• PRs
• in
• line

Thanks for the feedback! We've filed a ticket to fix this, so that the comment more closely resembles `gt ls` :slightly_smiling_face:

Cc <@U047KHH8JH3> who was thinking about updates to the Chrome extension

Hey Stefan! This is something we're exploring right now - we'll (hopefully) be able to provide more info about merge jobs in the near future

In the meantime, should I keep waiting or is there a chance there was a bug and I should cancel and restart?

Hmm can you share a PR number so we can look into this? cc <@U02RREE9GKH>

<https://app.graphite.dev/github/pr/snowflakedb/snapps/29304>

If it helps, I actually started the merge job a few minutes before the last PR in the merge had all the accepts necessary. So maybe that was causing problems?

Don't *think* that should be related, but good to know

Try restarting the job if you haven't already, either way we'll look into what could be happening

image.png

cc: <@U047KHH8JH3> re syntax highlighting

Ooh good suggestion - I believe you can submit feedback through the status page as well but we can configure this in the CLI too!

is there a way to somehow warn the user if they're using a merge commit on graphite branches? a couple of my colleagues run into nasty merge conflicts when accidentally mixing merging and rebasing, which has put them off graphite a bit.

one issue we encountered was that initially the feature branch is empty, so graphite will keep prompting you to delete it as it considers it fully merged

another is that you cannot merge branches to the feature branch using graphite dashboard. you can only queue the entire stack, including the feature branch. so no automatic restacking

and this one i'm not 100% sure of, but i think another issue was that once you merge a child to the feature branch (using github), graphite wasn't recognising this properly, so if you pulled the data from repo, you might encounter merge conflicts

Check out `gt user submit-body --include-commit-messages`! <https://graphite.dev/docs/configuration#user-level-configuration>

Hi Martin, thanks for the feedback! I'd like to make sure I understand the request properly and have a couple questions:
1. Today in the dashboard, we support a couple of similar (though not identical) filters. Do either of these help with your use case?
    a. `Includes team as reviewer`
    b. `Includes reviewer (directly or via team)`
2. If the above filters don't fit for your use case, is it because your GitHub team code review settings removes the team as an assignee after auto-assigning to children? I.e. for your team's PR's, the team itself is _not_ a specified PR reviewer, but you still want to see those files in your review queue?
3. Is your ideal state to have a filter that includes changes to any file you own via CODEOWNERS - directly or indirectly via team membership, independent of assignee? Do you have a need to configure this further, e.g. only for some teams you're part of, or is all-or-nothing sufficient?
appreciate the feedback!

Hi Jorge, my apologies I wasn’t not 100% correct when I said the dashboard. What I meant was when I’m in a PR doing a review that I’m missing an option to filter files which are relevant to the team I’m on based on `.CODEOWNERS` file.

ohh I see, thanks for the clarification! I like that idea - I'm planning to do some work soon to improve how we present codeownership info, and I'll run this by our designer to see how we can incorporate it. thanks again!

I thank you Jorge for taking my feedback. Looking forward to see what you guys come up with

Escape should remove focus from that textbox, i think this is a quick fix! it's been bothering me too...

yup, esc doesn’t work, and I can’t tab out either. Thank you!

Actually, tab should probably tab out of the input too for a11y reasons.

Cc <@U03Q89DJPL4> we should fix the shortcuts here

This is similar to (albeit, a less automagical) version of FB's `hg absorb`, — <https://sapling-scm.com/docs/commands/absorb|open source version>

it's something we're thinking about implementing our own version of!

we do have the `gt dash pr` command that does this separately from submit

Oooh, I didn’t know about that. That’ll be helpful all the same!

Weird though that `gt dash` isn’t documented o.O It just shows the same docs for `gt` but as `gt dash` on all the commands. Doesn’t mention `pr` at all.

Screenshot 2023-02-17 at 15.55.41.png

weird that it adds the `d` though... the library we use for arg parsing definitely has some issues

ah, yeah I think `-h` isn't a shortcut for help

but yeah, just repro'd your case... very strange

Oh. Derp- yeah, it says at the bottom `Unknown argument: h`.

it should still print the help though, of course

It does, just that part is beneath all of it.

cannot see it there though :thinking:  will try l8r

This is something we're working on getting better visibility on :slightly_smiling_face: In case you're curious, GitHub's API doesn't expose the actual reasoning why a PR is in "blocked" status, and so we're doing a bunch of work on our end to rederive this from branch protection rules! cc <@U047KHH8JH3> and <@U0486U7UUF3>

This is super helpful, and heard on all points for sure. Working on some overall design improvements to the PR page and will take note of this cc <@U02DCQ1C0G6> for viz

A quick note though - it looks like for the particular branch that you're working on (from what i can tell) does not have reviews set as a branch protection - in this case the badge would not show up as "ready to merge" unless the appropriate number of reviews are given

<@U04K5N14V9B> yep totally - we don’t have branch protection intentionally at the moment. I’d still like something to tell me if it’s been approved or not. In the Github UI it’s quite clear

Also noted!! One thing we're working on is pulling out the stack visualization into the main header of the PR. What this means is that you are able to navigate between PRs in a stack easily and can see *exactly* where in the stack you are at all times without the extra keystrokes (as you are describing from the graphite comment here). Curious to know if this design would help solve your issue?

Yes! that would definitely improve things, I think!

Basically if I’ve a way to move up and down the stack to gain the context without having to click multiple buttons, it would be great. A permanent stack sidebar would be great.

Something else, but this could be very nit picky - but having the oldest PRs at the bottom of the stack in the UI confuses the life out of me. I’d much rather have it sorted the other way round, with older PRs at the top and newer ones at the bottom. Working from top down makes a lot more sense to me

Trying to use the UI more so I can keep posting things that I’m finding frustrating, if that helps :slightly_smiling_face:

Ooh interesting, that's helpful to hear too! And yeah of course, we really appreciate your feedback/are actively taking note of it and definitely want to hear any other pain points or areas in which you think the UI could be improved as you continue to use it!!