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.

Sign up for GitHub

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

Jump to bottom

feat: add additional release links to the release #282

Merged
merged 1 commit into from
Sep 7, 2020
Merged

feat: add additional release links to the release #282

merged 1 commit into from
Sep 7, 2020

Conversation

Chumper
Copy link
Contributor

@Chumper Chumper commented Sep 1, 2020 •
edited
Loading

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

Closes #281

Screen Shot 2020-09-01 at 3 17 29 AM

@Chumper
Copy link
Contributor Author

Chumper commented Sep 1, 2020

I am not sure about the travis error. This seems to be a generic problem, maybe with NPM?
Locally I have 100% coverage.
Screen Shot 2020-09-01 at 3 21 30 AM

@Chumper
Copy link
Contributor Author

Chumper commented Sep 1, 2020 •
edited
Loading

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

@Chumper Chumper mentioned this pull request Sep 1, 2020
Closed
@felipecrs
Copy link

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

@gr2m
Copy link
Member

gr2m commented Sep 3, 2020

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 ...

@Chumper
Copy link
Contributor Author

Chumper commented Sep 3, 2020

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

@Chumper
Copy link
Contributor Author

Chumper commented Sep 3, 2020 •
edited
Loading

I found the error. I was using a feature that is not supported in node 10 and 12.
I was using Optional chaining like this property?.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.

gr2m
gr2m approved these changes Sep 6, 2020
View reviewed changes
Copy link
Member

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

Great work, thank you!

@gr2m
Copy link
Member

gr2m commented Sep 6, 2020

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

@Chumper
Copy link
Contributor Author

Chumper commented Sep 6, 2020 •
edited
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.

@felipecrs
Copy link

felipecrs commented Sep 6, 2020 •
edited
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 to false.

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

@Chumper
Copy link
Contributor Author

Chumper commented Sep 6, 2020

Sure, I will rework it to be false by default, but also to accept either bottom or top and adjust the code accordingly.

@Chumper Chumper force-pushed the add/releaseLinks branch 2 times, most recently from 162d9af to cdaf2a2 Compare September 7, 2020 02:12
@Chumper
Copy link
Contributor Author

Chumper commented Sep 7, 2020

@gr2m @felipecrs I have added the discussed changes. Allowed values are now false, top or bottom.

Default will be false.

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

gr2m
gr2m reviewed Sep 7, 2020
View reviewed changes
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide 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 releases
to the Github release body.

The option can be one of `false|"top"|"bottom"`.
The default is `false` to be backward compatible.

Closes semantic-release#281
@Chumper
Copy link
Contributor Author

Chumper commented Sep 7, 2020

Short and informative, I applied the changes.

gr2m
gr2m approved these changes Sep 7, 2020
View reviewed changes
Copy link
Member

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

Thank you Nils, this is great

@gr2m gr2m merged commit db2e9c8 into semantic-release:master Sep 7, 2020
@semantic-release-bot
Copy link
Collaborator

🎉 This PR is included in version 7.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@felipecrs
Copy link

This is awesome!

@Chumper Chumper deleted the add/releaseLinks branch September 7, 2020 18:57
@bubkoo bubkoo mentioned this pull request Nov 2, 2020
Open
@RomainOliva RomainOliva mentioned this pull request Mar 15, 2023
Merged
@davhdavh davhdavh mentioned this pull request Oct 4, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gr2m gr2m gr2m approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Append the release channels to the release
4 participants
@Chumper @felipecrs @gr2m @semantic-release-bot