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

Add release-drafter config #361

Merged
merged 7 commits into from Nov 11, 2020
Merged

Add release-drafter config #361

merged 7 commits into from Nov 11, 2020

Conversation

ssbarnea
Copy link
Member

This enables release-drafter which keeps a draft release at https://github.com/pytest-dev/pytest-html/releases with all merged changes.

This github action does not make any releases, but prepares the notes so the release manager can make an informed decision when tagging a new release using the mentioned page.

To see an example of how it looks, take a look at https://github.com/ansible/ansible-lint/releases or https://github.com/ansible-community/molecule/releases

@ssbarnea ssbarnea added the skip-changelog Can be missed from the changelog. label Oct 24, 2020
Copy link
Member

@gnikonorov gnikonorov left a comment

Choose a reason for hiding this comment

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

This is awesome @ssbarnea, thank you! I didn't know about this until now. I just had 2 questions about this

.github/release-drafter.yml Show resolved Hide resolved
.github/workflows/release-drafter.yml Show resolved Hide resolved
@ssbarnea
Copy link
Member Author

ssbarnea commented Nov 4, 2020

@gnikonorov @BeyondEvil Please give me an approve on this to enable it before creating the tag. Few minutes after we merge it we will have a nice draft in releases pages and we can tag it as 3.0.0a0 to mark the start of a new era.

Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

Isn't it better to move to something like Semantic Release instead (like discussed in another thread) to automate release entirely instead?

categories:
- title: 'Major Changes'
labels:
- 'major' # c6476b
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be breaking?

Copy link
Member Author

Choose a reason for hiding this comment

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

breaking does not deserve a label, and luckily we do not have one. Most changes marked as major are breaking but not all breaking labels are major. Sometimes major covers other changes.

We need to be careful to keep labels under control as I seen projects with tickets becoming like xmas tree and confusing users, most not even knowing what to pick. This list is short but enough in order to put changes in the right bucket (section). We can revise if needed but first let it settle so we get used to adding the right labels to the tickets.

BTW, My favorite feature is the release-drafter is that you can add the label or fix the PR title after the merge and at the next run, it will correct the draft. Still once a release is made, the bot will not touch the text, only humans can edit it.

There is also the aspect of "breaking" having negative connotations, something we may want to avoid.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair points.

Can you address my larger question about it isn't better to use Semantic Release and get all this PLUS automatic release? @ssbarnea

Copy link
Member Author

Choose a reason for hiding this comment

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

While I never used Semantic Release, two things come into my mind:

  • Decision to release should be left to humans, we do not want too many releases, and we didn't even add AI ;)
  • Reading https://github.com/semantic-release/semantic-release#commit-message-format -- i do not think that commit messages are safe to use for release notes, main reason is that once merged you cannot change it. It is too easy to merge wrong stuff. That is why almost everyone is using an other ways of dealing with release notes either using fragments, static changelog or fully outside git.

Altering commit message does retrigger the whole testing pipelines. That is why I prefer the idea of dealing with release notes on a OOB channel, as it is mostly bureaucracy, needed but it should not block or slowdown the PRs.

Please note that I did not propose to ditch current changelog (or the guidelines for changes). So mainly what we do now is to experiment how good is this approach for our project. I do expect to copy/paste the release notes and merge them to the changelog before each release, at least until we are confident that it works fine. I do not want to confuse existing users with something the cores are only exploring.

Copy link
Member

Choose a reason for hiding this comment

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

I'm inclined to agree with the two points @ssbarnea brings up above ( especially point 2 ). We could always move to semantic release if we find this too tedious

Copy link
Contributor

Choose a reason for hiding this comment

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

While I never used Semantic Release, two things come into my mind:

  • Decision to release should be left to humans, we do not want too many releases, and we didn't even add AI ;)

Well, it still is. Based on the the tag used in the commit message a release (major/minor/patch) will happen - or not.

What's the point in hoarding? If we merged a new feature, an update or a bugfix, why not release that right away?

  • Reading https://github.com/semantic-release/semantic-release#commit-message-format -- i do not think that commit messages are safe to use for release notes, main reason is that once merged you cannot change it. It is too easy to merge wrong stuff. That is why almost everyone is using an other ways of dealing with release notes either using fragments, static changelog or fully outside git.

We don't have to use commit messages as release notes. That's configurable. The commit messages only drive if a release happens and what type. What's being merged is controlled by a select few of us. If you feel wrong things are being merged we need to have a discussion about that and actions need to be taken to stop or at least minimize the risk.

Furthermore there are tools to make sure that commit messages follow a set pattern.

We use Semantic Release in a professional setting across many different repos. If it's good enough for that, it's good enough for OS.

However, with that said, one potential negative aspect is if there are too many hoops to jump through to contribute to the project, we may scare away new contributors.

But, like @gnikonorov said, nothing stopping us from trying out SR later.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is gold "one potential negative aspect is if there are too many hoops to jump through to contribute to the project, we may scare away new contributors." -- I seen in in the past, where maintainers succeeded in adding enough bureaucracy to deter most users. I am really glad we are aware of the risks.

This area is more of a tuning game, as long we adapt fast and learn from our mistakes we should be fine.

Copy link
Member

@gnikonorov gnikonorov left a comment

Choose a reason for hiding this comment

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

I had two questions/comments.

Also, release drafter will ignore unknown labels so we don't need to constantly sync release-drafter.yml with the labels list right?

.github/release-drafter.yml Show resolved Hide resolved
.github/release-drafter.yml Outdated Show resolved Hide resolved
Copy link
Member

@gnikonorov gnikonorov left a comment

Choose a reason for hiding this comment

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

Sorry, last request for changes on this @ssbarnea and then I think this is good to merge!

Also, can you please explain why we have the fix andbugfix labels under the Bugfixes section even though these don't exist in the label list. Can we remove them?

.github/release-drafter.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

Question: All these titles and labels, are they only concerning releases?

If not, I would add a build title/label to cover things like what we're doing right now - changing build/repo related stuff.

docs might also warrant its own section.

I like the Eslint convention: https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-changelog-eslint

Comment on lines +13 to +15
- 'fix'
- 'bugfix'
- 'bug' # fbca04
Copy link
Contributor

Choose a reason for hiding this comment

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

I question if we really need all three of these? I would say fix would suffice.

.github/release-drafter.yml Show resolved Hide resolved
Copy link
Member

@gnikonorov gnikonorov left a comment

Choose a reason for hiding this comment

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

Just wanted to say that I am OK approving once @BeyondEvil is happy with the changes as well. Worst case I figure we can always tune this as we use it and fix the drafts if needed, so I'm not too worried about getting it right in the first pass.

@BeyondEvil
Copy link
Contributor

Just wanted to say that I am OK approving once @BeyondEvil is happy with the changes as well. Worst case I figure we can always tune this as we use it and fix the drafts if needed, so I'm not too worried about getting it right in the first pass.

I've never used this, so I'm not sure I can sufficiently review it. But I trust you and @ssbarnea enough to give it my approval. From what I understand this is just admin stuff and nothing that will affect our users. Worst case we can always revert or change.

@ssbarnea ssbarnea merged commit 6fe35ca into master Nov 11, 2020
@ssbarnea ssbarnea deleted the gh/release-drafter branch November 11, 2020 19:26
@gnikonorov
Copy link
Member

Hey @ssbarnea I see that the draft made by this includes some very old PRs (e.g.: #136 ). How can we prevent them from appearing?

Also, do we need to update https://github.com/pytest-dev/pytest-html/blob/master/development.rst#releasing-a-new-version?

@ssbarnea
Copy link
Member Author

Yes, I seen it is bit of the mess. Ignore it and I will check what got it confused. Usually it should have included only changes merged after the last tag. Clearly it was more like all changes in that run, I did some manual cleanup.

For the moment ignore it and I will keep an eye on it to identify what went wrong and how it behaves in new runs.

While I am using it for more than two years on like 20-30 repos, this is the first time when it went nuts. Usually I got minor issues like same change listed twice in two sections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Can be missed from the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants