Skip to content

Don’t run update-pr-from-base-branch if personal token is undefined#3411

Closed
yakov116 wants to merge 2 commits intorefined-github:masterfrom
yakov116:pat-update-branch
Closed

Don’t run update-pr-from-base-branch if personal token is undefined#3411
yakov116 wants to merge 2 commits intorefined-github:masterfrom
yakov116:pat-update-branch

Conversation

@yakov116
Copy link
Member

  1. LINKED ISSUES:
    Resolves "Update Branch" button: show warning if PAT is undefined #2679

  2. TEST URLS:
    This pr just remove your token

  3. SCREENSHOT:
    None

PS: Regarding the unrelated lint, let me know if I should revert.

const [outOfDateContainer] = select.all('.completeness-indicator-problem + .status-heading')
.filter(title => title.textContent!.includes('out-of-date'));
const outOfDateContainer = select.all('.completeness-indicator-problem + .status-heading')
.find(title => title.textContent!.includes('out-of-date'));
Copy link
Member

Choose a reason for hiding this comment

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

🙌

@fregante fregante force-pushed the pat-update-branch branch from 980594a to fbf862a Compare July 28, 2020 13:23
@@ -54,7 +54,7 @@ export class RefinedGitHubAPIError extends Error {
}
}

Copy link
Member

@fregante fregante Jul 28, 2020

Choose a reason for hiding this comment

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

What if you export a function instead?

Suggested change
export async function expectToken(): string {
const {personalToken} = await settings;
if (!personalToken) {
throw new Error('Personal token required for this feature');
}
return personalToken;
}

And then call it in api.v4 too

Copy link
Member Author

@yakov116 yakov116 Jul 28, 2020

Choose a reason for hiding this comment

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

👍 (just stepped away from the computer. will do it a drop later)

Copy link
Member Author

Choose a reason for hiding this comment

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

I know why I didn't do that.
Since I do not want to throw, I just want to log the error.
The reason it does not error out by all the other features, is because logError takes over.

https://github.com/sindresorhus/refined-github/blob/15f81938ad41c270cac360a99905cc265be3c6d9/source/features/index.tsx#L57

WDYT?

Copy link
Member

@fregante fregante Jul 30, 2020

Choose a reason for hiding this comment

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

The reason it does not error out by all the other features, is because logError takes over.

That's the mechanism that we need to use. This error would be also caught by logError, won't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not, it throws.

@fregante fregante marked this pull request as draft July 30, 2020 12:02
@fregante fregante closed this in 6185565 Jul 31, 2020
@yakov116 yakov116 deleted the pat-update-branch branch August 3, 2020 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

"Update Branch" button: show warning if PAT is undefined

2 participants