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

DSP-618 Update release process #242

Merged
merged 12 commits into from
Sep 29, 2020
Merged

Conversation

kilchenmann
Copy link
Contributor

resolves DSP-618

@kilchenmann kilchenmann self-assigned this Sep 26, 2020
@kilchenmann kilchenmann added the chore Maintenance and build tasks label Sep 26, 2020
@kilchenmann
Copy link
Contributor Author

@tobiasschweizer and @mdelez This is nothing you can test. It runs on each merged PR into master branch. I tested it on a private repo and we already implemented it in dsp-ui and dsp-app
The workflow follows the new description in README and the Contribution documentation

@tobiasschweizer
Copy link
Contributor

@kilchenmann So if I understand correctly, this PR adds a release drafter that creates a release draft after each merged PR. This draft then can be turned into a prerelease.

So gren is stilled used to create the log?

CHANGELOG.md Outdated
@@ -2,7 +2,7 @@

## v1.0.0-rc.12 (22/09/2020)

#### Dependencies
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is that a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The corresponding PR has the label "Breaking"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you're right. The endpoint for permission has changed. But this is not a breaking change for js-lib users. I can update and do a new commit

- name: Get previous tag
id: previoustag
uses: "WyriHaximus/github-action-get-previous-tag@master"
- name: Update version
Copy link
Contributor

@tobiasschweizer tobiasschweizer Sep 28, 2020

Choose a reason for hiding this comment

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

EDIT Does this mean that after each merged PR, the version in package.json is changed?

Does the version number get adapted automatically now? If so, how would it be version controlled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With each release it takes the version number from git tag and updates the version in package.json automatically before publishing to npm.

Copy link
Contributor

Choose a reason for hiding this comment

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

but then the change in package.json would not be updated on GitHub, would it?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, I think now I get it: "0.0.0-PLACEHOLDER"

Copy link
Contributor

Choose a reason for hiding this comment

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

but then just looking at the files checked out from git, how would we know the current version?

Copy link
Contributor

Choose a reason for hiding this comment

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

hm, also true.

I am a bit concerned that we will mess up releases by mistyping the release tag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main problem with updating the version in the git repository: it needs always an extra commit. It needs a PR and a review because we can't push into master branch directly.

I followed angular/components and how they did it. e.g.
https://github.com/angular/components/blob/master/src/material/package.json

Copy link
Contributor

Choose a reason for hiding this comment

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

or is there a check that compares to the previous version?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we've based a lot of our stuff off how the Angular team works so this seems like an okay method to me. There is the possibility of typo's, but that's true in a lot of places. It's pretty convenient not having to open a PR just to change a number so I think we should try out this method and we just need to be careful when typing the release tag. We also know, thanks to me being a noob a couple weeks back, that we can stop the process from releasing a version that was mistyped/has an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @mdelez
And btw we use this workflow already in dsp-app and knora-api since the day we use docker. It builds the docker image with the version defined in git-tag.

@kilchenmann kilchenmann reopened this Sep 28, 2020
@kilchenmann
Copy link
Contributor Author

kilchenmann commented Sep 28, 2020

@kilchenmann So if I understand correctly, this PR adds a release drafter that creates a release draft after each merged PR. This draft then can be turned into a prerelease.

Yes exactly. The draft can be turned into a prerelease or a real release.

So gren is stilled used to create the log?

No, we use release-drafter. Gren is still here, because we can probably use it to update CHANGELOG.md. But this will be part of another PR.
I was using gren to update the changelog manually.

@kilchenmann kilchenmann merged commit 646c58c into master Sep 29, 2020
@kilchenmann kilchenmann deleted the wip/DSP-618-update-release-process branch September 29, 2020 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Maintenance and build tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants