NotificationsYou must be signed in to change notification settings
Fork142
Star488

feat: add additional release links to the release#282

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

gr2m merged 1 commit intosemantic-release:masterfromChumper:add/releaseLinks

Sep 7, 2020

Merged

feat: add additional release links to the release#282

gr2m merged 1 commit intosemantic-release:masterfromChumper:add/releaseLinks

Sep 7, 2020

Conversation

Copy link

Contributor

Chumper commentedSep 1, 2020•
edited by heartsbot
Loading

A new optionaddReleases has been added.
Setting this option will instruct the plugin to append all
additional releases to the Github release on the top.

Closes#281

Copy link

ContributorAuthor

Chumper commentedSep 1, 2020•
edited by heartsbot
Loading

I am not sure about the travis error. This seems to be a generic problem, maybe with NPM?
Locally I have 100% coverage.

Chumper force-pushed theadd/releaseLinks branch from861f2dd to614d748Compare

September 1, 2020 01:25

Copy link

ContributorAuthor

Chumper commentedSep 1, 2020•
edited by heartsbot
Loading

@gr2m @pvdlg Could you help with the travis errors here? I am not sure on how to fix them.

Chumper mentioned this pull request

Sep 1, 2020

Publish NPM URL on GitHub releasessemantic-release/semantic-release#1617

Closed

Copy link

felipecrs commentedSep 1, 2020•
edited by heartsbot
Loading

I believe you can add the closure ofsemantic-release/semantic-release#1617 to this PR as well. (Closessemantic-release/semantic-release#1617)

Copy link

Member

gr2m commentedSep 3, 2020•
edited by heartsbot
Loading

I've restarted the CI builds, but the same tests failed again. I'll need some time to investigate, and I don't know when I'll have that time right now, I'm afraid. We didn't have builds for a while, so it might be a change in some of the dependencies ...

Copy link

ContributorAuthor

Chumper commentedSep 3, 2020•
edited by heartsbot
Loading

I will also have a look tomorrow, I just have never seen that the travis files are in a different repository.

Chumper force-pushed theadd/releaseLinks branch from614d748 to7f1b4b1Compare

September 3, 2020 11:48

Copy link

ContributorAuthor

Chumper commentedSep 3, 2020•
edited by heartsbot
Loading

I found the error. I was using a feature that is not supported in node 10 and 12.
I was usingOptional chaining like thisproperty?.id which is not supported in node version below 13.

That is what you get when you are not mainly progamming in node.

@gr2m This can now be reviewed and merged.

Chumper force-pushed theadd/releaseLinks branch from7f1b4b1 to40db841Compare

September 3, 2020 11:55

gr2m approved these changes

Sep 6, 2020

View reviewed changes

Copy link

Member

gr2m left a comment•
edited by heartsbot
Loading

Choose a reason for hiding this comment

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

Great work, thank you!

Copy link

Member

gr2m commentedSep 6, 2020•
edited by heartsbot
Loading

Actually I wonder if we should add the links to the bottom, not the top. I think it will work better, because changelog is the primary information I expect in release notes, links to sibling releases is meta information

Copy link

ContributorAuthor

Chumper commentedSep 6, 2020•
edited by heartsbot
Loading

I would argue against that, the release information is always short enough so that you can skip those lines and get to the changelog.
The changelog could contain a few more lines and at least in my case I am more interested in the releases than the changelog.

If you give me another hour, I can change this so that you could do:addReleases: <top|bottom|false>

Then then choice is up to the user.

Copy link

felipecrs commentedSep 6, 2020•
edited by heartsbot
Loading

I agree that the release links should be at the bottom and not in top. Because of the same reason of@gr2m.

@Chumper btw, the choice is a great addition since this is a breaking change. Unless we leave it default tofalse.

I suggest to include the option, leave it default to "bottom" (or whatever you decide), and then move ahead with this breaking change.

Copy link

ContributorAuthor

Chumper commentedSep 6, 2020•
edited by heartsbot
Loading

Sure, I will rework it to be false by default, but also to accept eitherbottom ortop and adjust the code accordingly.

Chumper force-pushed theadd/releaseLinks branch 2 times, most recently from162d9af tocdaf2a2Compare

September 7, 2020 02:12

Copy link

ContributorAuthor

Chumper commentedSep 7, 2020•
edited by heartsbot
Loading

@gr2m @felipecrs I have added the discussed changes. Allowed values are nowfalse,top orbottom.

Default will befalse.

Please review again and merge if you think this will work.

Chumper force-pushed theadd/releaseLinks branch fromcdaf2a2 toc47c0b6Compare

September 7, 2020 02:14

gr2m reviewed

Sep 7, 2020

View reviewed changes

README.md OutdatedShow resolvedHide resolved

feat: add additional release links to the release

dea56d8

A new option `addReleases` has been added.Setting this option will add links to other releasesto the Github release body.The option can be one of `false|"top"|"bottom"`.The default is `false` to be backward compatible.Closessemantic-release#281