Graphite Community

and I don't see a direct link to install the app unless you <https://graphite.dev/docs/slack-integration|log in to Graphite>, so I can't just send an install link to the workspace admin

this is a one-time hurdle obviously, but I assume others will run into it as well

thanks for the flag! we currently push folks to install the app via graphite so that we can associate your slack and graphite accounts

here is <https://slack.com/apps/A02T59R5J4V-graphite?tab=more_info|our app directory link>, which you should be able to send to an org admin — then you can follow the instructions in the guide to enable notifications!

we’re also working on getting our app approved so that it can be listed in the store — more updates on that coming soon!

ah, the app directory link looks like it also has you make a graphite account first - it goes through the same flow of enabling in the notification settings

i think we’re going to need to be approved by the store before this works properly :confused:

Yeah, I'm not sure how to install it at all actually

Our workspace admin approved it for install

yeah i’m looking into it now and it turns out that ‘direct install’ is an approved feature only

our slack app review is in progress though! so hopefully we’ll see that come through soon

what do you see when you attempt to install the app via your notification settings?

that same error still, oddly enough
<https://graphite-community.slack.com/archives/C02DRNZELUU/p1681232914175619?thread_ts=1681232909.179199&amp;cid=C02DRNZELUU>

even though the workspace admin has gone and approved it for install

how long does the app approval take? I'm also happy to just wait if that's not too far away

we’ve been told the review process can take 4-6 weeks :see_no_evil:

we submitted it pretty recently, so a good chunk of that is left unfortunately

let me see if there’s a workaround we can figure out for you in the meantime though!

ok, thanks! I appreciate it :slightly_smiling_face:

ugh there doesn’t seem to be a way to do this without being fully listed :disappointed: is there any chance you can get your org admin to sign up for graphite lol

also if you could get some info from them around what your org’s app install permissions are~ that would be really helpful for us going forward! we haven’t seen this issue before so im suspicious that there’s some configuration we’re not handling well

is there any sort of direct link to install the app, other than the flow through graphite's settings?

unfortunately not today, but once we’re approved we can utilize <https://api.slack.com/start/distributing/directory#direct_install|direct install>

image.png

<@U0526TLNAR1> if you select text and just paste from clipboard we will do the same

would it be possible to switch it (maybe a config) to be more like github?

(also more like most other apps, which use cmd+k for this?)

slack is sort of the odd app out here... to the point they have a little flyover if you hit cmd+k telling you how to do it the "Slack way"

Linear &amp; Superhuman let you do either of these - we should probably support both as well :slightly_smiling_face:

An example: imagine you have two stacked PRs, `feature-a` and `feature-b` :
```* 0379b46e8 (HEAD -&gt; feature-b) Work for feature B
* bd1a7fa86 (feature-a) Work for feature A
* 3a7cb2819 (master) Base foo```
Right now, you’d make two PRs:
• Merging `feature-a` into `master` 
• Merging `feature-b` into `feature-a`

In this alternative world, we’d make a `base/xxx` branch for each PR:
```* 0379b46e8 (HEAD -&gt; feature-b) Work for feature B
* bd1a7fa86 (feature-a, base/feature-b) Work for feature A
* 3a7cb2819 (master, base/feature-a) Base foo```
And the two PRs would be:
• Merging `feature-a` into `base/feature-a`
• Merging `feature-b` into `base/feature-b`

If the first PR for `feature-a` is landed onto master, the PR for `feature-b` remains intact and untouched - the UI on github still only shows that the PR has one commit
```* fdda0084e (HEAD -&gt; master) Work for feature A
| * 0379b46e8 (feature-b) Work for feature B
| * bd1a7fa86 (base/feature-b) Work for feature A
|/
* 3a7cb2819 Base foo```

The other benefit is if multiple users are working off of a stack: for ex, a stack of backend PRs (aka `feature-a`) with a frontend eng working on a stack of frontend PRs (aka `feature-b`) off the current tip of the backend feature stack. Right now, if `feature-a` is updated and force pushed, `feature-b` ends up in this wonky state containing all of the commits from `feature-a`

I worked with phab for a long time, and when I left for another company, I built my own CLI that did this `base/xxx` thing - until i found graphite :slightly_smiling_face: both flows have their pros and cons, and the `base/xxx` approach is admittedly really difficult to retrofit to work with Github’s merge-centric world. But I would be beyond intrigued to play around with a mode of the graphite CLI that did manage its own explicit base branches

This would be absolutely incredible to have support! Really curious on the graphite team’s feedback here :slightly_smiling_face: ^^

Another side effect of the current method using shared base branches is that updating only `feature-a` on the bottom of a stack falsely causes `feature-b` to temporarily display that it has changes from earlier version of `feature-a`. This is fine as long as you restack and push the rest of your stack, but if you use github `CODEOWNERS`, all code owners of `feature-a` will be incorrectly added to the review of `feature-b` and must be removed manually :sad_parrot:

Any thoughts on this approach?

cc <@U02RREE9GKH> who likely has the strongest opinions here

Really cool idea! One of our primary goals has always been super-solid interop with non-Graphite GH workflows.

It’s still interoperable, with quirks - when you merge in GH UI, you have to adjust your base branch to master. Inconvenient for sure, but so many benefits for a stacked workflow. I almost see this method as an extension for graphite power-users who want the stacks to work seamlessly for them. It doesn’t have to be a hard cutover, I imagine the two modes could coexist with a setting in the graphite cli. Would the team ever configure supporting a second mode like this, or the ability to add custom behavior at stack submit time?

Definitely something I'll consider -- our more likely first path will be to do these rebases automatically, to be transparent

Ooh fascinating - like if someone updates `feature-a`, a graphite job with automagically rebase `feature-b` in the background and push?

That would definitely help a ton for a lot of these use cases. I can see one side effect: we’d automatically re-trigger CI for all of the PRs on top of the stack. I can see how some cos actually want that as desired behavior, but if your CI is relatively expensive, some cos might want to minimize CI retriggers on just code changes to the PR itself rather than rebases

That's a great point, appreciate the flag

:wave: Any updates in thoughts or feature development here?

We've been thinking about the evolution of the workflow along this dimension, but in very early stages so far. Our current thought is that we can likely provide better UX on stacks for teams that are all using Graphite (like the way in which you mention here) at the cost of making the experience for team members still viewing PRs on GitHub less clear than it currently is. Your suggestion to allow configuration between the two modes is likely how we would do something like this. Again, very early stages — cc <@U02DGG322G5> or <@U02DGFZ1H0D> for viz in case you have anything to add or any research questions.

Separately, I see that in the thread I was also discussing the auto-rebase stuff — we've built that and experimented with it internally but have put it on hold due to how it felt to actually use day-to-day. We're looking at read-only solutions to the "needs a rebase" case for the near future (e.g. some sort of notice/explanation).

Wow, you made my evening! :slightly_smiling_face: very happy to hear it’s evolving, but I acknowledge it’s a huge change. This today is one of our biggest pain points in working with stacks. More than happy to help in any way that I can - bouncing ideas around, elaborating on our use cases, testing janky first iterations, you name it

Also, is there a better place for feedback like this? My goal would be for this to hopefully be updated (or declined, if that were your decision) and not lost in the stream. Can I add a Github issue or something?

cc our designer <@U02DCQ1C0G6>. We can create a ticket to investigate!

As a follow-up, you can add feeeback to our feature request board <https://app.graphite.dev/changes-requested|here> - you're able to upvote features you see there or create your own!

Should I just put stuff there and ignore this channel in the future?

feel free to post here! it's good to be able to chat about the exact issue you're facing and potential solutions/workarounds, and improves visibility!

promise it's not annoying, we really appreciate the feedback :slightly_smiling_face: 

<@U02E95J4648> built a prototype version of butterfly bot/herald during a hackathon! Will let him respond regarding the future of this

Ahhh I remember seeing this!! This would be super fun, we'll take note and play around with it!

Imagine being able to do this with ReviewMojis too

Internal phab did too but the syntax was impossible to remember. I had to have a bookmark for it because I wanted to use it but couldn’t save it. 

It was the regex of code review. 

Maybe there could be an option to tie it to a different PR?

If you can enable the "Automatically delete head branches" setting on GitHub, this shoudn't be an issue — but we are also planning to add a banner for this case! cc <@U02DCQ1C0G6>

<https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-the-automatic-deletion-of-branches>

asked here: <https://graphite-community.slack.com/archives/C02DRNL3HQU/p1680557976380769?thread_ts=1680300991.225619&amp;cid=C02DRNL3HQU>
:grin:

we can probably make this more discoverable, thanks for the feedback!

glad you're enjoying reviewmojis :grin: 

There's a GitHub setting for this! <https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-the-automatic-deletion-of-branches>

last time I checked, if I enable that, then Graphite doesn't pick up that the branch was merged

hmm, that is not expected behavior — we have this setting enabled and don't see that, as well as some of our other users

Do you use any external merge tools on your repository?

Just did a series of merges with that setting on, followed by a `gt rs -r` and got prompted to remove the 3 branches locally

k, ty. maybe I had something misconfigured on my end

++ my workaround is to push drafts to remote and then delete them locally -- can always `downstack get` them later!

Ooh thanks for the rec! We'll flag this :saluting_face:

thanks for flagging — this was a mistake and has been fixed! you should be able to approve multiple times now :slightly_smiling_face:

<@U04HQKF7TSN> is working on both of these as part of a major Insights refresh we’re shipping soon :slightly_smiling_face:

yay! Thanks <@U04HQKF7TSN> could you share more about what's in the works for team insights? I'm looking at various options for solving our issues but I'd love for this tab to be able to do it all for us

1. More metrics around efficiency of reviews:
    a. Time waiting for reviews
    b. Number of review cycles till merge
    c. Time taken to respond to review requests
2. New UI
3. More scalable &amp; faster backend, so you should now be able to view large orgs at a single time, and results are being pre-calculated so should be quite fast

I'd love to know what you are looking for &amp; we can think about how to address your usecases

All of the above is what we are looking for. plus:

1. ability to select a github team as a source of which contributors are selected. 
2. Some way of saving the selection criteria for sharing (url?)

IIRC we do have #2 planned, but not in the immediate term. But seems totally reasonable &amp; not that crazy to add. So will look into it :slightly_smiling_face:

For 1, that should also be doable, but syncing teams from GitHub is a little challenging as they can be recursive. How big are these teams? We have a user selector, so if the teams are not too large you should be able to select people individually and then share through suggestion #2

Cool! yeah we have teams of 5 to 10 at the bottom of the graph rolling up to teams 50 to 100 that we would like to look at.

So you can see why we need at least one #1 or #2 for this to be useful for us

<@U054BJU7RNG> -- this should now be available to use!

You can use `s` and then navigate with arrow keys

Regarding #1, improving our web interface is our biggest priority right now!

Regarding #2, while not directly addressing your question, have you tried using Graphite slack notifs?

Regarding #2: yes we did, but they do not help us keeping the GitHub notifications clean that we need for other projects were Graphite is not used yet.

Thanks so much for the feedback <@U052F92LUBZ>, and most importantly thanks so much for giving Graphite a try with your team :)
As Aryaman said, we're currently doing a lot of improvements on the UI and correctness is one of our highest priorities. 
As for number 2, we'd suggest giving Graphite notifs a try! They're a bit more concentrated/actionable than GitHub notifs, so definitely less spammy. 
<@U054EU5DL2Y> oh man, totally hear you here. We don't have a fix for this at the moment, but we'll keep it in mind!

Thank you for getting back to me! And it's good to hear that that's what you are working on. Personally, I'd much rather get more stability/correctness than features :sweat_smile:

The problem with Graphite notifs is, that the GitHub notifs are still there. And I don't (yet) dare turning those off as I am not yet sure how reliable Graphite is and I don't want to miss anything.

Still, overall I am really enjoying Graphite :heart:

Screenshot 2023-04-21 at 16.15.55.png

Screenshot 2023-04-21 at 16.11.41.png

I agree! We are planning to move this formatting to make it match `gt ls`

<@U02RREE9GKH> – if you're planning to match `gt ls` you may want to consider matching what's "up" and "down"! A new user on my team was confused about what's "upstack" because of the ordering of the GH comment.

yep, that's exactly what I meant :slightly_smiling_face:

A bit of a blast from the past, but some people on our team are still mixing up "downstack" and "upstack" because of the GH comment rendering. Any plans to ship updated formatting?

I am working on this now that launch is done :slightly_smiling_face:

As `gt` forwards unknown subcommands to `git`, the completion should also include Git completions if the Git completion is available.
But only where it is actually forwarded of course. `gt log` will not be forwarded as far as I have seen, except for the hard-coded `gt log long`.

It would be nice if `gt log long` would forward additional parameters to `git` instead of failing.
For example `gt log long -30` to only show the first 30 lines.

The `include-commit-messages` configuration has been super helpful!

However, the fact that it never includes the first line of the commit message makes it less useful for our use case since our commit messages are usually one-liner bodies. From the docs:
&gt; Note that if you only have a single commit on your branch, the first line of the message (i.e. its title) will not be included as this is already the default for the name of the PR.
Would be great to have an option to disable this behavior, even if it's hidden away somewhere. Thanks! :pencil2::gear:

100% - comment permalinks exist and <@U04AP0M9AES> is planning to add a way to copy the link to each comment soon.  We’re also working on a better solution for rendering in-line comments that get lost due to rebasing in the right place in Graphite that will make this problem go away entirely!

That’s great to hear!

For your second point, just as a reference, sometimes I get caught up in merge conflicts after an interior-edge in the gt stack chain gets merged down. In the rebasing the upstack commits don’t seem to be cherry-picked so they all get renamed.

It seems like if the upstack branch got reset to the new downstack parent and the new commits on the upstack branch were cherry-picked in the original commit order, things would go smoother.

I thought that was how rebasing worked, but something about the cascading updates in `gt stack restack` seems to break that down?

I do this process manually sometimes because it’s a headache to replay old commits on top of themselves.

The alternative is to just merge in the downstack branch but that feels risky and prone to possibly overwriting new changes in the upstack branch.

I know that was a lot so I’m happy to clarify anything / give an example if that’s helpful

As an added benefit, the github “view changes since you last reviewed” are preserved when using explicit cherry-picking since the commit hashes are all the same

Commit hashes are never the same if you cherry-pick, except if you pick a committee that is already there and thus nothing is done. The hash of a commit in Git has the parent commits encoded and thus the complete history of that commit up to the initial commit. That's also why force-pushing branches (except for in PRs) is a big no-go, as it involves major work everyone else has to do.

<@U04HUGX3DMF> Does this match what you are saying? <https://graphite-community.slack.com/archives/C02ENKC0A9X/p1682571240136819?thread_ts=1678203599.084219&amp;cid=C02ENKC0A9X>

Yeah I think that is it <@U054ZV6BNG0>. The experience I’m having is your example:
&gt; I don’t want to overwrite, because that would erase `b3`
&gt; I pick rebase, this tries to rebase `b1, b2, b3` on top of `b1', b2'`
Maybe the `git rebase --onto` solution is what I’m expecting the restack to be doing behind the scenes

Yep, that's what I expected it to do initially, but I think I now understand better why it's not. With the `rebase --onto` flow, I have to manually define which commits to completely skip, because I understand that `b1, b2` is equivalent to `b1', b2'`, but I guess maybe git/gt cannot really know that. Maybe it can though... Not sure :sweat_smile:

In the "Files" panel it would be nice to haver an option to "mark as (re)viewed" a folder and all its children. We have generated code that we commit in our repos and semi-frequently it can be 20-30 files that really have no use for a detailed review - so they just end up being noise, being able to quickly hide/view/ignore them would be ideal. We don't put them as a separate diff in the stack because we want each PR to be atomic, we dont want to merge something that result in a broken main branch.

Gotcha, thanks for the note - we're working on improvements to overall workflow and we'll think about this as well!

I believe you should be able to override it, or provide a secondary template of your choosing!
<https://graphite.dev/docs/using-a-pr-template>

I don’t want to use any template at all

that doc page seems to be mostly about how GitHub handles various templates though, no?