Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Backport: Blame: fix some issues with the stream (#63865)#63929

Merged
craigfurman merged 2 commits into
5.5.xfrom
backport-63865-to-5.5.x
Jul 22, 2024
Merged

Backport: Blame: fix some issues with the stream (#63865)#63929
craigfurman merged 2 commits into
5.5.xfrom
backport-63865-to-5.5.x

Conversation

@camdencheek

@camdencheek camdencheek commented Jul 18, 2024

Copy link
Copy Markdown
Member

Contributes to SRCH-738

Notably, this does not yet identify the root cause of SRCH-738, but it does identify and fix some confounding bugs. It's possible that these actually also cause some of the issues in SRCH-738, but I wanted to at least push these to dotcom, where we can reproduce some of the weirdness. At the very least, it doesn't explain the auth errors being reported.

(cherry picked from commit d91fab3)

Test plan

CI

Contributes to SRCH-738

Notably, this does not yet identify the root cause of SRCH-738, but it
does identify and fix some confounding bugs. It's possible that these
actually also _cause_ some of the issues in SRCH-738, but I wanted to at
least push these to dotcom, where we can reproduce some of the
weirdness. At the very least, it doesn't explain the auth errors being
reported.

(cherry picked from commit d91fab3)
@cla-bot cla-bot Bot added the cla-signed label Jul 18, 2024
// Test needs more time to teardown
test.setTimeout(testInfo.timeout * 3000)
test('file popover', async ({ page, sg }) => {
test.slow()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated conflict resolution

@camdencheek camdencheek marked this pull request as ready for review July 18, 2024 23:49
@camdencheek camdencheek requested review from a team and DaedalusG and removed request for a team July 18, 2024 23:49
@bahrmichael

Copy link
Copy Markdown
Contributor

@craigfurman @DaedalusG This PR now contains tests fixes from main and is ready to merge. Let me know if there's anything I need to do to have it merged.

@craigfurman

Copy link
Copy Markdown
Contributor

@bahrmichael you should be able to merge it now that its approved. If that's not the case, please ping here again and we can merge it 🙂

@DaedalusG I'm a bit lost after recent release/backport changes, do the release team actually have to approve backports into release branches? https://www.notion.so/sourcegraph/Sourcegraph-Backport-Tool-211d005248bb4b588b4fe8401381e71c implies no - developers can approve backport PRs made by the bot themselves and merge them, according to that. Manual cherry-picks like this of course need to be reviewed by any other org member, because that's just how github works, but AFAIK it doesn't have to be the release team. Holler if I'm wrong!

Zooming out, it doesn't make sense IMO for the release team to review all backports - to do so would imply we have reviewer-level context on every change in the company - but this comment is a question about the process today rather than one about how we might want to change it.

@bahrmichael

Copy link
Copy Markdown
Contributor

@bahrmichael you should be able to merge it now that its approved. If that's not the case, please ping here again and we can merge it 🙂

@craigfurman GitHub doesn't allow me to merge, because "The base branch restricts merging to authorized users." Can you hit the button?

@craigfurman craigfurman merged commit 556b880 into 5.5.x Jul 22, 2024
@craigfurman craigfurman deleted the backport-63865-to-5.5.x branch July 22, 2024 09:43
@craigfurman

Copy link
Copy Markdown
Contributor

Sorry for the delay @bahrmichael, TIL that is the shape of the merge protection rule. Ironically I can't see it, not being a repo admin 😂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants