Skip to content

Conversation

@ZempTime
Copy link
Contributor

@ZempTime ZempTime commented Apr 12, 2023

Once merged, please bump the mintor version to 2.1.0

During the large, recent refactor by @jemmyw the behavior where changing a pr title would relink a feature to the new refnum contained in the title was inadvertently changed.

This reintroduces that prior behavior by prioritizing the title over the url when looking up a feature in response to a webhook update of a pull request.

I'm unsure if this constitutes a bump from 2.0.1 -> 2.0.2, or a minor bump of 2.0.1 -> 2.1.0.

@ZempTime
Copy link
Contributor Author

This introduces the minimal change. One question I have for reviewers is: should we prioritize the title over the url everywhere?

Other places:

export async function recordFromPrLink(pr: IPullRequestLink) {
return recordFromFinders([
["PR URL", () => recordFromUrl(pr.url)],
["PR title", () => recordFromReferenceNum(pr.name)],

export async function recordFromPrForLinkFragment(
pr: PrForLinkFragment & PrCommitFragment
): Promise<LinkableRecord | null> {
const finders: Finder[] = [
["PR URL", () => recordFromUrl(pr.url)],
["PR Title", () => recordFromReferenceNum(pr.title)],

export async function recordFromSimplePullRequest(pr: SimplePullRequest) {
return recordFromFinders([
["PR URL", () => recordFromUrl(pr.html_url)],
["PR Title", () => recordFromReferenceNum(pr.title)],

I'm unsure what unintended effects changing these could have.

@percyhanna percyhanna requested a review from jemmyw April 12, 2023 17:48
Copy link
Collaborator

@jemmyw jemmyw left a comment

Choose a reason for hiding this comment

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

A minor version change would be fine

@percyhanna percyhanna merged commit c1239da into main Apr 13, 2023
@percyhanna percyhanna deleted the DEVELOP-1678--github-extension-updating-pr-name-relink-pr branch April 13, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants