Graphite Community

image.png

:wave: i'm a neovim user and engineer at graphite; i actually have a ticket filed to look into this as well.
I think what I want to do is change the file name from `EDIT_DESCRIPTION` to `EDIT_DESCRIPTION.md` - this way, neovim (and other editors?) will syntax highlight for Markdown automatically

let me look now, it feels like a 2 character change and as a new engineer here i might as well get familiar with the CLI :slightly_smiling_face:

looks like a thing to discuss with team, so i kicked off a thread and i'll let you know

<@U05447TKZKP> would you mind expanding on what you mean by this

&gt; But while just editing a body, this is meaningless and more disturbs greatly:
i think i know what you mean but it might be more powerful/clear in your own words why you think it's not necessary to be in Git commit mode here

Because I am not editing a commit message there.
So the yellow coloring to restrict the subject line and the red background that reminds to keep a blank line between subject and body is pointless and just hurts the eye.
In that use-case I'm just editing the body of the PR, so the commit-modes just makes no sense at all.

I'm having trouble remembering who it was (due to our 30 day slack retention), but I believe at some point a relatively active user in the community asked us to use `EDIT_DESCRIPTION` as the name for this file because they specifically wanted this behavior — I think we can make it configurable. Sending to the channel in case the user in question sees this

If you also talked about it in a thread, you can find it in your threads view by scrolling, or if you bookmarked the message, you could also find it there.
Both are not affected by the 90 (not 30) day retention, you just cannot find it by search. :slightly_smiling_face:

whoa TIL! i am confused why if that's the case, permalinks don't work

unfortunately there's no context in this PR description <https://github.com/withgraphite/graphite-cli/pull/1238/files>

but the date is july 29, 2022 so maybe if we scroll back there

As I said, look in your threads view if you started a thread about it.
That way I also got at 2 year old threads and continued them in another workspace.

In the channel and thus also permalinks it does not work beyond the 90 days

I have so many threads, but am about 5 months back right now and still scrolling!

Only in the threads view and bookmarks view

what about this idea:
• rename it to `GRAPHITE_PR_DESCRIPTION.md`
• in vim if someone wants this custom, they can force it to be gitcommit with `set ft=gitcommit` on buffer enter matching that filename
• otherwise most people will get the default Markdown (which imo is more "correct")

i don't know what editor people use to edit the PR descriptions typically, but i imagine the default is `EDITOR=vi`

we can add a quick doc callout for how to config Vim for this

and calling it `GRAPHITE_PR_DESCRIPTION` lets people just target this specific case

vim8
```autocmd BufNewFile,BufRead GRAPHITE_PR_DESCRIPTION.md set ft=gitcommit```
neovim
```vim.api.nvim_create_autocmd({ "BufNewFile", "BufRead" }, {
  pattern = "GRAPHITE_PR_DESCRIPTION.md",
  command = "set ft=gitcommit",
})```

Does maybe `-e` make a difference?
I didn't try that option so far.
But if it means that the title and body are edited within one `vim` session and there the subject should be separated from the body by a blank line just like for regular commit usage,
then it might make sense to use `EDIT_DESCRIPTION` in that case and something else in other cases.

&gt; Does maybe `-e` make a difference?
sorry which program are you recommending `-e` for?

I have a PR open to fix, pending review! <https://github.com/withgraphite/graphite-cli/pull/1411>

Just to make sure you got me right.
If you use `gt bs -e`, which I did not use so far, what is the content of the file you edit?
If it is PR title, then empty line, then body, it should probably be like it is right now in that specific case.

it prompts you at the CLI for title edits like so:

then it pops you into `$EDITOR` for description:

it interactive updates title, updates PR desc via EDITOR, then syncs changes -&gt; Github

```  -e, --edit         Edit PR fields inline. If --no-interactive is true, this is
                     automatically set to false.                       [boolean]```

it's a way to edit the PR metadata of your current branch in one shot

Yeah, I've read that.
Probably interpreted "inline" wrongly.
I thought inline within the editor editing

yeah i think we could probably improve that help string cc <@U02RREE9GKH> if you have thoughts

Ah yeah, that was me. And thank you for the command for how to fix this up on my end!

Yeah. I'm pretty novice overall at Vim configuration :slightly_smiling_face:

latest version is available and should fix this one! <https://graphite-community.slack.com/archives/C02DKGQCSDQ/p1683856237207479>

Screenshot 2023-05-09 at 5.05.06 PM.png

graphite-bug.gif

Thanks for flagging - filing a ticket! (<@U0486U7UUF3> for mergeability?)

We currently don't support the `Require branches to be up to date before merging` branch rule which is probably why you're seeing this. There are some complexities on our side with knowing when the branch is not on the most up to date target branch but this is something we're aware of and hope to tackle at some point!

Just showing a message on merge attempt (I'm guessing GitHub API returns some sort of error to you) would be helpful. The first time I saw this I thought it was just Graphite being broken.

I agree we should have better error messaging here. Thanks for flagging!

Screenshot 2023-05-10 at 9.13.45 AM.png

:ladybug: If a diff is both approved and has requested changes, it doesn't show up anywhere on the dashboard. The “Returned to You” shows (1) but the list itself is empty.

hey nico i've actually fixed that one already :slightly_smiling_face: but i'm blocked doing a release of the extension today

as soon as i get unblocked i will do the release

hey <@U04P4R523AM>! Sorry this is happening, if the changes are being committed to the branches correctly i don't see why these branches shouldn't be updating

Thanks! I've tried a few things with no luck

Things I've tried today:
• `gt branch untrack &amp;&amp; gt branch track` to reset tracking
• `gt repo init --reset` to reset the state of everything, then retracking all my branches
• `brew remove withgraphite/tap/graphite` and re-running our company script to re-install
• Opening new branches/PRs to see if that works
I'm still stuck in a state where I add a commit to an existing branch/PR, the following happens:
• Running `gt stack submit -u` - Each branch shows No-op and doesn't update
• Running `gt stack submit` or `gt stack submit -u --always` tries to create new PRs
I'm pretty stuck right now. Any help would be appreciated!

Ok some how this started working now! I now see links to my PRs when running `gt l` , and running `gt stack submit -u` updates things correctly :tada:

One thing I tried this morning was to move over to a Github PAT

hmm, just seeing this, not sure which of these fixes did it for you, but glad things are working now! lmk if you have any further issues

Thanks! I think it was either switching to a PAT, or waiting for the github outage or both :slightly_smiling_face:

yeah, in the past for auto branch names i've only allowed `-` and `_` chars, and converted spaces to `-` and non A-Za-z0-9 to either `-` or empty string

<https://graphite-community.slack.com/archives/C02DRNL3HQU/p1683812181146989>

Graphite - GitHub outage ​*[Monitoring]*​ Graphite is currently affected by GitHub's active incident and experiencing degraded performance.
 ​*Affected:*​
CLI, Dashboard, Notifications
 View incident button

Graphite - GitHub outage ​*[Resolved]*​ This incident has been resolved.
 ​*Affected:*​
CLI, Dashboard, Notifications
 View incident button

Today it seems like there's a PR that's not showing up in any of my views, in either the count or the listing.

Hm thanks for flagging - are you still seeing this behavior?

Does not appear to be happening today. It had started before the big GitHub outage last week, I'd probably been seeing it for a few weeks I would guess, on Safari and Brave.

I'm trying to do a `gt be` on a mid-stack branch, but what I get after changing the sole line from `pick` to `e` is:
&gt; Stopped at f0155df918...  Add SpecInfo#getAllSharedInitializerMethods()
&gt; You can amend the commit now, with
&gt; 
&gt;   git commit --amend
&gt; 
&gt; Once you are satisfied with your changes, run
&gt; 
&gt;   git rebase --continue
&gt; ERROR: Command failed with error exit code 128:
&gt; git switch getAllSharedInitializerMethods
&gt; 
&gt; fatal: cannot switch branch while rebasing
&gt; Consider "git rebase --quit" or "git worktree add".

May be related: merging a stack with rebase option checked now stops after merging the first PR with error: “Graphite couldn’t merge this PR because it had conflicts with the trunk branch.” Seems like a regression because this was working before.

I had the same, but it still merged the second PR without rebasing for me.

Interesting - thanks for the call out guys!

We're looking into it :slightly_smiling_face:

hey folks! sorry you're running into this issue — if you could each DM me the PR URL I can look into it for you

Actually — quick update here, one of our teammates has found what we believe to be the root cause and we're working on a fix! Can still verify that this was in fact the issue you were running into

hey <@U054ZV6BNG0>! this base branch (master) is actually the branch that the whole stack will be merging into. `authzd/platform-roles` is the actual branch which the current branch is based off of though

i think we can do a better job of indicating this

CleanShot 2023-05-12 at 10.33.07@2x.png

CleanShot 2023-05-12 at 10.33.59@2x.png

<@U02PSKGK4TS> do you get any browser domain security type errors in the console?

```<ps://github.com/thebrowsercompany/arc/assets/922840/df889609-a493-432d-9d44-b8157db78738> 404
Image (async)
Tk @ main.ae6bb1b3.js:2
Rk @ main.ae6bb1b3.js:2
Qk @ main.ae6bb1b3.js:2
Il @ main.ae6bb1b3.js:2
Al @ main.ae6bb1b3.js:2
rl @ main.ae6bb1b3.js:2
J @ main.ae6bb1b3.js:2
R @ main.ae6bb1b3.js:2```
And the response summary:
```Request URL: <https://github.com/thebrowsercompany/arc/assets/922840/df889609-a493-432d-9d44-b8157db78738>
Request Method: GET
Status Code: 404 
Remote Address: 140.82.113.3:443
Referrer Policy: strict-origin-when-cross-origin```

<@U02PSKGK4TS> hm, curl also 404s. i suspect we are not auth'd in to view the image:

```╰ curl -is <https://github.com/thebrowsercompany/arc/assets/922840/df889609-a493-432d-9d44-b8157db78738>
HTTP/2 404
server: <http://GitHub.com|GitHub.com>
date: Fri, 12 May 2023 19:57:39 GMT```

what is the actual repro step?

1. you attach an image via *github's* PR view
2. you try to view it in *graphite*
3. it fails?

IIRC if you upload an image via graphite, we put it in our own S3/CDN bucket

This was someone else’s comment. I bet they added that image from the GitHub UI. 

after digging in a bit more, the images uploaded via Github's UI cannot be displayed on Graphite due to browser security rules

because graphite cannot send an auth cookie when requesting the image (due to not being on the `<http://github.com|github.com>` domain), the image 404s instead of redirecting you to the correct CDN asset

i attempted to `&lt;iframe&gt;` the image, but Github (rightfully) blocks this behavior via the Content-Security-Policy on the image

my current best idea is that we can detect the 404 error and render a "Sorry, you tried to view a private image, click here to open it in a new tab" component in place of the image. which isn't great, but better than a mysterious broken image

Aren’t pull request bodies available from the github api? I would be surprised if they didn’t have a workaround for a situation like that

i think <@U02E95J4648> found a workaround

Following up here: we’ve started using image upload urls from the comment/description `bodyHTML` rather than `bodyMarkdown` field - the same as what GitHub does for their VSCode extension. The `bodyHTML` contains presigned upload urls which load on Graphite. We deployed this late Friday, and it’s now live on the site. Please let me know if you find edge cases where this isnt working

there are issues with image flickering, which I will fix shortly

CleanShot 2023-05-15 at 14.55.56@2x.png

well that is extremely weird… im looking into it!

i’ve identified that this is the result of a bad caching change that we recently landed — this should be fixed soon!

more context if its helpful. i asked a teammate with identical slack permissions to go through the same flow and he had no issues enabling; it worked first try :weary:

i tried on two browsers, arc and chrome, with same result

could you DM me your github handle? i’ve got a hunch as to what is going on here

scroll-position-bug.mov

Ah this is a known issue, we're working on fixing the re-rendering here. Thanks for flagging!

Yep, everything is just loading like the status checks, comments and files :thinking:

Hmm that's strange - things look alright from my end. Any logs or errors you're hitting on the console?

Will look into this! Sorry for the inconvenience

Ffs... I think I switched the authentication method by mistake when looking through the settings earlier in the day and that's why everything was broken. Switch back to github app auth and now everything seems fine

Ah yeah...funny you mention this because I'm currently working on making our onboarding process more robust (including what happens when you're kicked out due to auth issues)

Nice, looking forward to it :saluting_face:

&gt; Sorry for the trouble :sad_parrot:
Never apologize!!

Hey <@U03FNT1FLP5>! Thanks for reporting - lemme look into this

Hey <@U04K5N14V9B> seems to still not be working. I’m going to try uninstalling and reinstalling the app but apparently now only workspace owners can do that? :thinking_face: am I looking in the wrong place?

I got our workspace set up to allow others to install apps. Working now!

Amazing! Glad you were able to resolve this :slightly_smiling_face:

hmm, can you dm me a PR that this happens for? will look into it

Hey! So this is actually due to a weird behavior of GitHub where reply-only reviews don't show up in the timeline (and so we don't show them in our timeline either, because their timeline API response matches their timeline).  We actually have a workaround for this for replies created in Graphite where we create an empty review so that it does show up in the timeline, which is why your first reply does actually show up in the timeline here.

Obviously, we have the data in the thread even though it isn't in the timeline, so I wonder if we might be able to start looking for cases like this and injecting them into our timeline view.  cc <@U04AP0M9AES> and <@U03Q89DJPL4>

sorry about that! we forgot to update that instance

Graphite - GitHub outage ​*[Investigating]*​ Graphite is currently affected by GitHub's active incident and experiencing degraded performance.
 ​*Affected:*​
CLI, Dashboard, Notifications
 View incident button

Ah this is really odd and definitely not expected behavior - ccing <@U04HQKF7TSN> for visibility here!

This shouldn’t be the case, can you share a PR where this happened? I have a plan to fix these issues more systematically next week. 

<https://app.graphite.dev/github/pr/vivid-labs/vivid/2242/Reuse-Component-Update-DB>

We got it merged by going to the url directly, but I hope it's still helpful

Could you share a screenshot of your filters? For the section you expected to see it in

It might be an issue with how the filter are set up. The issue with that specific PR was that the repo got one approval (from someone else), but I also reviewed and requested changes. When the author of the PR sent it back, it didn't show up on my feed:
• wouldn't pass filter#1 since the PR did have minimum required approvals
• Would also not pass filter #2 since PR did have an approval
• Unclear about filter #3 — maybe that's the one that should pull the PR in this scenario

Thanks for the debug info! I'm going to spend some time debugging these issues next week and hope to resolve it then. :slightly_smiling_face:

This should be in your "Waiting for review" section

<@U03RGJEDWH5> looks like you are still on the old filtering logic. Ok if I migrate you to the new filters? I’ll take a back up of your current filters in case you want to revert back. 

<@U03RGJEDWH5> what is your github username

Let me know if you continue to see these issues

how can we migrate ourselves to the new filters?

I can do it for you (give me your github handle), or just delete all your sections &amp; use the reset button that shows after that. We really should make this easier.

Thanks for reporting this bug! (I will point out that the feature you're using is our stack merge feature, and not the merge queue that we've announced recently)

Can you DM me the pr repo and number that this happened on? It should be using the message from the dialog — I'll take a look.

DMed you. My bad on confusing this with the merge queue!

No worries! It's a little confusing and we're moving away from the "queued" language for this feature, but we know it's still in use in a couple places

I've identified a bug that is likely causing this, rolling out the fix later today — i believe it should fix your case, but if you're still running into this after today, let me know!

:crying_cat_face: devastated… I believe we’re addressing the line breaking discrepancy soon

Did this happen today (just now)? You can DM me the pr repo/# to confirm, but if it was today, it looks like you selected "merge" as a strategy and the repo doesn't allow merge commits.

Yeah, looks like you're right. I haven't ever paid much attention to the merge strategy before - does that get defaulted when the PR is created perhaps? I just changed the rules on this particular repo today, after creating the PRs but before attempting to land them.

github's api does actually vend us a default option that should match whatever you last used in that repo, but it's slow to update + also doesn't respect repo settings much of the time.  we can do better here, namely checking the repo rules and overriding their default if necessary

the default is per person per repo, not per pr, it's just not reliable -- e.g. we've noticed that whenever a new eng starts at our company their default shows up as merge as well (we also disallow merge commits)

Got it. Thanks for the explanation, that's helpful.

Very strange - will have someone look into this!

Following up here - we aren't able to expand/highlight files that are too big (as stated in the message for this file here) - how big is the file that you were trying to view?

hmm, my expectation was that 16 changed lines was not too large to display, but perhaps you’re loading the entire file?

Looking in the browser console I see that `update-reminder-digest` is returning a 400, if that helps.

Sorry you're running into this! Will look into it

Hey <@U058B0B8YF4>! Are you still having issues here?

I am; here's a quick loom that shows the issue <https://www.loom.com/share/1aa2c9d5f87641ff9fad3b33099acb86>

Please let me know if there's some way to get you more info. I looked at the response headers but didn't see a requestId or anything obvious.