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

ci: Check if valid title is used in PR #989

Merged
merged 2 commits into from
Aug 14, 2020

Conversation

piotradamczyk5
Copy link
Contributor

@piotradamczyk5 piotradamczyk5 commented Aug 12, 2020

Fixes #814

Test Plan

How do we know the code works?

  • There is additional trigger which checks PR title
  • PR generate release notes and append to release_notes.md

Checklist

  • Documented
  • Unit tested
  • release_notes.md updated

@bootstraponline
Copy link
Contributor

.github/workflows/after_pr.yml

What are the tradeoffs of generating release notes after PRs are merged vs before they're merged? It might be nice to review the generated release notes as part of the pull request.

@piotradamczyk5
Copy link
Contributor Author

piotradamczyk5 commented Aug 12, 2020

.github/workflows/after_pr.yml

What are the tradeoffs of generating release notes after PRs are merged vs before they're merged? It might be nice to review the generated release notes as part of the pull request.

It generates release notes based on PR title after merging, if we do it before, then we will have merge conflicts anyway.
If you would like to check generated release notes just check the title of PR 😃

Also please check file flank-scripts/src/main/kotlin/flank/scripts/ci/releasenotes/ConventionalCommitFormatter.kt
to see how conventional commits prefix is translated to release notes:

    startsWith("feat") -> replaceFirst("feat", "New feature".markdownBold())
    startsWith("fix") -> replaceFirst("fix", "Fix".markdownBold())
    startsWith("docs") -> replaceFirst("docs", "Documentation".markdownBold())
    startsWith("refactor") -> replaceFirst("refactor", "Refactor".markdownBold())
    startsWith("ci") -> replaceFirst("ci", "CI changes".markdownBold())
    startsWith("test") -> replaceFirst("test", "Tests update".markdownBold())
    startsWith("perf") -> replaceFirst("perf", "Performance upgrade".markdownBold())

@bootstraponline
Copy link
Contributor

that makes sense! I'm looking into creating a Flank app and then adding the token from that.

@bootstraponline
Copy link
Contributor

bootstraponline commented Aug 12, 2020

Bot accounts are specific to GitHub Apps and are built into every GitHub App.

https://docs.github.com/en/developers/apps/differences-between-github-apps-and-oauth-apps

There's some way to get a bot account from a GitHub App and have the access token associated with the app. I think we'll need to figure this out. We're trying to move away from user accounts/personal access tokens. There are security and API rate limit concerns with using user accounts for repo automation.

It might make sense to fork flank into a new org, experiment there, and then we'll bring back the learnings to the production Flank repo.

@bootstraponline
Copy link
Contributor

@piotradamczyk5
Copy link
Contributor Author

piotradamczyk5 commented Aug 12, 2020

https://docs.github.com/en/developers/apps/authenticating-with-github-apps#http-based-git-access-by-an-installation

I created https://github.com/apps/flank but am not sure where the token is. 🤔

Maybe here is the answer ?
I do not know how it works

Edit:
Also, nice tutorial about settings it up (Method 4)

And helper page

@bootstraponline
Copy link
Contributor

I'm super excited for the conventional commits automatic release notes generation. Thanks for helping me think through a few different possibilities. If we generate the release notes after every commit, that'll add a lot of noise in the git history of the project.

After reflecting, I think the best way to generate release notes is directly before a release. This matches the angular convention where this practice originated from.

https://github.com/angular/angular/commits/master/CHANGELOG.md

This also resolves the problem of having to provide a highly privileged token that interfaces with a protected branch. What do you think?

@piotradamczyk5
Copy link
Contributor Author

https://github.com/angular/angular/commits/master/CHANGELOG.md

I'm super excited for the conventional commits automatic release notes generation. Thanks for helping me think through a few different possibilities. If we generate the release notes after every commit, that'll add a lot of noise in the git history of the project.

After reflecting, I think the best way to generate release notes is directly before a release. This matches the angular convention where this practice originated from.

https://github.com/angular/angular/commits/master/CHANGELOG.md

This also resolves the problem of having to provide a highly privileged token that interfaces with a protected branch. What do you think?

Ok I will split this PR into 2 separate.

  • This will just check if conventional commit title is right
  • second one I will change a logic to generating release notes from last release to current one and make some PR with them

Is it ok?

@bootstraponline
Copy link
Contributor

Sounds good!

@piotradamczyk5 piotradamczyk5 changed the title feat: Added option to generate release notes automatically ci: Check if valid title is used in PR Aug 13, 2020
@piotradamczyk5 piotradamczyk5 force-pushed the #814-auto-generation-of-release-notes branch from 2ea5a14 to d3eee6a Compare August 13, 2020 11:07
@piotradamczyk5 piotradamczyk5 marked this pull request as ready for review August 13, 2020 11:07
@piotradamczyk5 piotradamczyk5 force-pushed the #814-auto-generation-of-release-notes branch 2 times, most recently from b085ee2 to 7c4172e Compare August 14, 2020 09:59
jan-goral
jan-goral previously approved these changes Aug 14, 2020
release_notes.md Outdated
@@ -3,6 +3,7 @@
- [#987](https://github.com/Flank/flank/pull/987) Flank Error Monitoring readme addition ([sloox](https://github.com/Sloox))
- [#990](https://github.com/Flank/flank/pull/990) Fix: exclusion of @Suppress test. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#988](https://github.com/Flank/flank/pull/988) Add versions description command for ios and android. ([adamfilipow92](https://github.com/adamfilipow92))
- [#948](https://github.com/Flank/flank/pull/948) CI changes: Check if valid title is used in PR. ([piotradamczyk5](https://github.com/piotradamczyk5))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe its should be
#989
?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks 👍

@piotradamczyk5 piotradamczyk5 merged commit 0fc8aaa into master Aug 14, 2020
@piotradamczyk5 piotradamczyk5 deleted the #814-auto-generation-of-release-notes branch August 14, 2020 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce release_notes.md merge conflicts
4 participants