Improve branch getter function #4190
Conversation
| @@ -96,7 +95,10 @@ async function init(): Promise<false | void> { | |||
| </a> | |||
| ); | |||
|
|
|||
| (await elementReady('#branch-select-menu', {waitForChildren: false}))!.closest('.position-relative')!.after(link); | |||
| const branchSelector = await elementReady('#branch-select-menu', {waitForChildren: false}); | |||
| branchSelector!.closest('.position-relative')!.after(link); | |||
There was a problem hiding this comment.
Just a lint, no changes in this file
| @@ -51,9 +53,9 @@ function checkoutOption(remote?: string, remoteType?: 'HTTPS' | 'SSH'): JSX.Elem | |||
| className="copyable-terminal-content" | |||
| > | |||
| <span className="user-select-contain"> | |||
| {remote && `git remote add ${remote} ${connectionType[remoteType!]}${getPRHeadRepo()!.nameWithOwner}.git\n`} | |||
| git fetch {remote ?? 'origin'} {getCurrentBranch()}{'\n'} | |||
There was a problem hiding this comment.
Usage of getCurrentBranch in PRs was ambiguous (there are 2 branches) and not it completely throws when used. It's best to use the splitter above. It can be made into its own helper later.
| } | ||
|
|
||
| if (!unslashedCommittish) { | ||
| console.trace('Branch could not be determined'); |
There was a problem hiding this comment.
This can help with future debugging, but it can be noisy.
|
|
||
| const [, _user, _repo, type, unslashedCommittish] = pathname.split('/'); | ||
| if (!type) { // Root | ||
| console.trace('Branch could not be determined'); |
There was a problem hiding this comment.
A dom query attempt could be added here, maybe similar to what's in #4187
Probably it'll need to be sync due to its usage in a setter of GitHubURL
| t.is(getCurrentCommittish( | ||
| '/typescript-eslint/typescript-eslint', | ||
| 'typescript-eslint/typescript-eslint: Monorepo for all the tooling which enables ESLint to support TypeScript' | ||
| ), undefined); |
There was a problem hiding this comment.
Currently it makes no attempt to query the DOM. #4187 could help
| t.is(getCurrentCommittish( | ||
| '/typescript-eslint/typescript-eslint/blame/chore/lerna-4/docs/getting-started/README.md', | ||
| 'History for docs/getting-started/README.md - typescript-eslint/typescript-eslint' | ||
| ), 'chore'); // Wrong, but |
There was a problem hiding this comment.
Actually this page is the only one that has an RSS feed, it could be fixed.
|
Untested but should be done, for the current iteration. It's best we merge a solution soon so that most feature resume working |
|
It might work in GHE too. Let's push it to production and see |
|
Final notes:
|
|
Merged too early. Undone. Releasing version anyway |
Replaces #4187
Fixes many
WIP