Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Take changelog from remote if provided #12455

Merged
merged 2 commits into from
Sep 27, 2016
Merged

Take changelog from remote if provided #12455

merged 2 commits into from
Sep 27, 2016

Conversation

XVincentX
Copy link
Member

The following PR should be the missing part to make #11940

@msftclas
Copy link

Hi @XVincentX, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@XVincentX
Copy link
Member Author

@joaomoreno Last part is here!

@@ -108,7 +108,7 @@ class Extension implements IExtension {
return this.local.changelogUrl;
}

return ''; // Hopefully we will change this once the gallery will support that.
return this.gallery && this.gallery.assets.changelog;
Copy link
Member

Choose a reason for hiding this comment

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

This was the wrong place. You should do it in the hasChangelog and getChangelog. While you're at it, just delete the readmeUrl and changelogUrl; the former was an old implementation which I forgot to delete.

Copy link
Member

Choose a reason for hiding this comment

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

Actually no...

Make those two getters private and use them in the getReadme and getChangelog methods to get a hold of the URL.

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.

Awesome dude, almost there!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 61.186% when pulling 769b96a25c2d921554d23d4cbab721d53b500553 on XVincentX:master into 4e09528 on Microsoft:master.

@XVincentX
Copy link
Member Author

Addressed in 154a45c @joaomoreno

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 61.19% when pulling 154a45c8104f465aee0feaa29abbd43326f8e26d on XVincentX:master into 4e09528 on Microsoft:master.

@joaomoreno joaomoreno self-assigned this Sep 22, 2016
@joaomoreno joaomoreno added this to the September 2016 milestone Sep 22, 2016
@XVincentX XVincentX closed this Sep 26, 2016
@XVincentX
Copy link
Member Author

@joaomoreno I think I made a mistake here while rebasing my fork.
Is it still possible to make this happen within September release?

@XVincentX XVincentX reopened this Sep 27, 2016
@msftclas
Copy link

Hi @XVincentX, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@XVincentX
Copy link
Member Author

Ok I managed to reopen it.

@joaomoreno joaomoreno closed this Sep 27, 2016
@joaomoreno joaomoreno reopened this Sep 27, 2016
@msftclas
Copy link

Hi @XVincentX, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 61.228% when pulling 80c9eb9 on XVincentX:master into c844c65 on Microsoft:master.

@joaomoreno joaomoreno merged commit d31fc79 into microsoft:master Sep 27, 2016
@joaomoreno
Copy link
Member

Thanks @XVincentX, great work!!

@XVincentX
Copy link
Member Author

I can't wait to see it live!

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants