Skip to content

Add githubBranch flag to control the branch for GitHub relative links#474

Merged
joaomoreno merged 3 commits intomicrosoft:masterfrom
JeffreyCA:jeffreyca/github-branch
Aug 11, 2020
Merged

Add githubBranch flag to control the branch for GitHub relative links#474
joaomoreno merged 3 commits intomicrosoft:masterfrom
JeffreyCA:jeffreyca/github-branch

Conversation

@JeffreyCA
Copy link
Member

Fixes #468

Currently if repository in package.json is set to a GitHub URL, all the relative links in README.md get converted to absolute links but they always refer to the master branch. This change adds a flag githubBranch which allows the user to override this assumption without having to use to override baseImagesUrl and baseContentUrl.

The baseImageUrl and baseContentUrl options take precedence over githubBranch, so doing something like: vsce --githubBranch main --baseImagesUrl http://blah will only change non-image relative links to stem from the main.

Copy link
Member

@joaomoreno joaomoreno left a comment

Choose a reason for hiding this comment

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

Can you write a test that actually tests the option, ie makes sure the links to GitHub have the branch name in the URL? The new test simply tests that overwriting of options works.

@joaomoreno joaomoreno self-assigned this Aug 9, 2020
@JeffreyCA JeffreyCA requested a review from joaomoreno August 9, 2020 15:28
Copy link
Member

@joaomoreno joaomoreno left a comment

Choose a reason for hiding this comment

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

Now you've gone too far. Please remove all the changes to package.json and yarn.lock as they don't seem necessary.

Edit: oh I missed your comment. No, I'd rather we keep TypeScript locked and just use the ternary operation. Sorry for that! Can you revert and simply keep the test addition?

@JeffreyCA JeffreyCA requested a review from joaomoreno August 10, 2020 18:27
@joaomoreno joaomoreno merged commit bf11c09 into microsoft:master Aug 11, 2020
@joaomoreno
Copy link
Member

Thanks! 🎆

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.

Extension marketplace readme renderer assumes you're using branch called master

2 participants