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

#892 Create release workflow #910

Merged
merged 33 commits into from
Jul 30, 2020
Merged

#892 Create release workflow #910

merged 33 commits into from
Jul 30, 2020

Conversation

piotradamczyk5
Copy link
Contributor

@piotradamczyk5 piotradamczyk5 commented Jul 22, 2020 •
edited
Loading

Fixes #892

Test Plan

How do we know the code works?

Release could be done using github actions RELEASE workflow.
Check docs/release_process.md and flank-scripts/README.md for more info

Checklist

  • Documented
  • Create Empty template for each step on github action
  • Get and set tag as environment variable
  • Update bugsnag
  • Delete old snapshot
  • [Gradle] Build flank (just invoke it using github actions)
  • [Gradle] Upload to bintray(just invoke it using github actions)
  • Rename flank jar
  • Hub remove old release
  • Remove old tag
  • if snapshot version release snapshot
  • if not snapshot version release stable
  • Sync bintray to maven central
  • Release notes updated

@piotradamczyk5 piotradamczyk5 self-assigned this Jul 22, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2020 •
edited
Loading

Codecov Report

Merging #910 into master will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #910      +/-   ##
============================================
+ Coverage     81.29%   81.34%   +0.05%     
- Complexity      663      664       +1     
============================================
  Files           204      204              
  Lines          3683     3683              
  Branches        544      544              
============================================
+ Hits           2994     2996       +2     
+ Misses          394      393       -1     
+ Partials        295      294       -1

bootstraponline
bootstraponline reviewed Jul 29, 2020
View reviewed changes
flank-scripts/README.md Outdated
Release Flank on GitHub

Options:
`--input-file`      Path to release file
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of  , consider using a GitHub Flavored Markdown Table

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

@piotradamczyk5 piotradamczyk5 marked this pull request as ready for review July 29, 2020 11:49
@piotradamczyk5 piotradamczyk5 force-pushed the #892-release_on_github_actions branch from b6d5110 to 0bb5cdd Compare July 29, 2020 14:46
jan-goral
jan-goral reviewed Jul 30, 2020
View reviewed changes
Copy link
Contributor

@jan-goral jan-goral left a comment

Choose a reason for hiding this comment

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

I am not sure if putting Command related functions to dedicated files is necessary as long as those commands and functions are simple and take only a few lines of code. For example:

  • DeleteOldReleaseCommand.kt - DeleteOldRelease.kt
  • DeleteOldSnapshotCommand.kt - DeleteOldSnapshot.kt
  • SyncMavenCommand - SyncMaven.kt

flank-scripts/src/main/kotlin/flank/scripts/exceptions/FlankScriptsExceptionMappers.kt Outdated
import flank.scripts.utils.toObject

fun <V : Any, E : FuelError, E2 : Exception> Result<V, E>.mapClientError(transform: (E) -> E2) = when (this) {
is Result.Success -> Result.Success(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just?

is Result.Success -> this

flank-scripts/src/main/kotlin/flank/scripts/utils/Serialization.kt

private val json by lazy { Json(JsonConfiguration.Stable) }

fun <T> T.toJson(serializationStrategy: SerializationStrategy<T>) = json.stringify(serializationStrategy, this)
Copy link
Contributor

@jan-goral jan-goral Jul 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It is possible to do that without passing SerializationStrategy. There is an inline version of stringify which can resolve SerializationStrategy implicit.

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 tested it and it does not work properly for all cases. Also using it will force use to use only classes with @Serializable annotation.
The issue is that
Can't locate argument-less serializer for class Object. For generic classes, such as lists, please provide serializer explicitly.
Also, I tested it with Any but with same effect ...

flank-scripts/src/main/kotlin/flank/scripts/release/hub/RemoveOldTag.kt Outdated
import flank.scripts.exceptions.mapClientError
import flank.scripts.exceptions.toGithubException

fun deleteOldTag(tag: String, username: String, password: String) =
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename file to DeleteOldTag.kt to match to function name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

flank-scripts/src/main/kotlin/flank/scripts/release/updatebugsnag/UpdateBugSnagCommand.kt Outdated

override fun run() {
runBlocking {
withContext(Dispatchers.IO) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if the changing dispatcher is necessary here.
Anyway, runBlocking can take CoroutineContext as the first argument, so withContext is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

flank-scripts/src/main/kotlin/flank/scripts/release/hub/DeleteOldTagCommand.kt Outdated

override fun run() {
runBlocking {
when (val response = withContext(Dispatchers.Default) { deleteOldTag(gitTag, username, token) }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if withContext(Dispatchers.Default) is necessary here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@piotradamczyk5 piotradamczyk5 force-pushed the #892-release_on_github_actions branch from 0bb5cdd to ca6a7b4 Compare July 30, 2020 14:03
@piotradamczyk5 piotradamczyk5 force-pushed the #892-release_on_github_actions branch from ca6a7b4 to 265cb38 Compare July 30, 2020 14:08
jan-goral
jan-goral previously approved these changes Jul 30, 2020
View reviewed changes
@jan-goral
Copy link
Contributor

Great job 💪

adamfilipow92
adamfilipow92 approved these changes Jul 30, 2020
View reviewed changes
Copy link
Contributor

@adamfilipow92 adamfilipow92 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 👍

@piotradamczyk5 piotradamczyk5 merged commit ab834c4 into master Jul 30, 2020
@piotradamczyk5 piotradamczyk5 deleted the #892-release_on_github_actions branch July 30, 2020 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamfilipow92 adamfilipow92 adamfilipow92 approved these changes

@jan-goral jan-goral jan-goral left review comments

@bootstraponline bootstraponline Awaiting requested review from bootstraponline

@pawelpasterz pawelpasterz Awaiting requested review from pawelpasterz

Assignees

@piotradamczyk5 piotradamczyk5

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move Flank release job from Bitrise to GitHub Actions
5 participants
@piotradamczyk5 @codecov-commenter @jan-goral @bootstraponline @adamfilipow92