-
-
Notifications
You must be signed in to change notification settings - Fork 585
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
Update release script to do signed releases #305
Conversation
if a signing ID is set in release_config.yaml Also set the release text to the relevant changelog entry
@@ -145,21 +153,43 @@ if [ $dodist -eq 0 ]; then | |||
# We haven't tagged yet, so tell the dist script what version | |||
# it's building | |||
DIST_VERSION="$tag" npm run dist | |||
|
|||
`dirname $0`/scripts/changelog_head.py > latest_changes.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like it won't work because of the pushd "$builddir"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, where is changelog_head.py?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, added changelog_head. And yeah, this should work since it's taking the path the script itself is executed with (which will be node_modules/matrix-js-sdk/ because of https://github.com/vector-im/riot-web/blob/master/release.sh#L12) and executing changelog_head relative to that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except that at this point you are in the build dir, so node_modules won't be found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I believe this works now.
@@ -145,21 +153,43 @@ if [ $dodist -eq 0 ]; then | |||
# We haven't tagged yet, so tell the dist script what version | |||
# it's building | |||
DIST_VERSION="$tag" npm run dist | |||
|
|||
`dirname $0`/scripts/changelog_head.py > latest_changes.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we doing anything here to ensure that the riot-web changelog references or links to the equivalent matrix-react-sdk changelog (and similarly js-sdk changelog)? At the least, we might want to include non-versioned links to the latest_changes for those projects. And slightly better would be links to the right version of latest_changes for the deps...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The release script is common to js-sdk and react-sdk though, and this would all be riot-web specific logic to insert js-sdk and react-sdk changelogs.
lgtm modulo comments, assuming it works :D |
so we know the relative path is right
if a signing ID is set in release_config.yaml
Also set the release text to the relevant changelog entry