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

Collect release notes from commits #322

Merged
merged 11 commits into from
Nov 17, 2021
Merged

Conversation

lolski
Copy link
Member

@lolski lolski commented Nov 16, 2021

What is the goal of this PR?

We've updated the release notes script to collect from commits rather than milestones. It will collect the commits between the last release up to the specified commit being released.

Additionally, it's been rewritten in Kotlin.

What are the changes implemented in this PR?

  1. Update the script for collecting release note from commits, which works as follows:
    • given C1, the commit that we want to release, collect L, the list of commits that we want to add to the release note:
      • if this is the first release ever, L would contain all the commits from C1 down to and including the first commit in the repo
      • if this is not the first release:
        • find C2, the commit of the release that immediately precedes this release
        • L would contain all the commits from C1 down to but excluding C2
    • write PR description or commit message for each commit in L into the release note
  2. Rewrite in Kotlin

fun main(args: Array<String>) {
val buildWorkspaceDirEnv = "BUILD_WORKSPACE_DIRECTORY"
if (System.getenv(buildWorkspaceDirEnv) == null) throw RuntimeException("Not running from within Bazel workspace")
val workspaceDirectory = System.getenv(buildWorkspaceDirEnv)
Copy link
Contributor

@vmax vmax Nov 17, 2021

Choose a reason for hiding this comment

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

@lolski could be collapsed with previous lines:

Suggested change
val workspaceDirectory = System.getenv(buildWorkspaceDirEnv)
val workspaceDirectory = System.getenv("BUILD_WORKSPACE_DIRECTORY")
?: throw RuntimeException("Not running from within Bazel workspace")

Copy link
Member Author

Choose a reason for hiding this comment

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

I see. Fixed in fa97b42.


val createTokenEnv = "CREATE_NOTES_TOKEN"
if (System.getenv(createTokenEnv) == null) throw RuntimeException("$createTokenEnv environment variable is not set")
val githubToken = System.getenv(createTokenEnv)
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski could be collapsed with previous lines:

Suggested change
val githubToken = System.getenv(createTokenEnv)
val githubToken = System.getenv("CREATE_NOTES_TOKEN")
?: throw RuntimeException("CREATE_NOTES_TOKEN environment variable is not set")

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in fa97b42.

val repo = args[1]
val version = args[2]
val toBeReleased = args[3]
val releaseTemplateFile = Paths.get(workspaceDirectory, args[4])
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski destructuring syntax could probably be used here

Suggested change
val releaseTemplateFile = Paths.get(workspaceDirectory, args[4])
val (org, repo, version, toBeReleased, releaseTemplatePath) = args

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in fa97b42.

}
}

val releaseNote = """
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski could use better-looking indentation if you call .trimIndent() on it:

val releaseNote = """
        Install & Run: http://docs.vaticle.com/docs/running-typedb/install-and-run
        ## New Features
        ${features.map { feature -> createCommitNote(feature) }.joinToString("\n")}

        ## Bugs Fixed
        ${bugs.map { bug -> createCommitNote(bug) }.joinToString("\n")}

        ## Code Refactor
        ${refactors.map { refactor -> createCommitNote(refactor) }.joinToString("\n")}

        ## Other Improvements
        ${others.map { other -> createCommitNote(other) }.joinToString("\n")}
""".trimIndent()
Files.write(releaseTemplateFile, releaseNote.toByteArray(UTF_8))

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that but it's not working properly.

This format

    val markdown = """
        Install & Run: $installInstruction
        
        ## New Features
        ${features.map(::createCommitNoteMd).joinToString("\n")}
        ...
    """.trimIndent()

resulted in an overindented first few lines:

        Install & Run: http://docs.vaticle.com/docs/running-typedb/install-and-run
        
        ## New Features
        - **Implement rule negation validation to prevent contradictions**
  To properly support negation in rules, we must validate that rules cannot contradict themselves and reach a stable final solution, we require that rules containing negations do not recurse through the negation - this is called stratified negation.This PR implements the commit time cycle detection required to ban cyclical rules through negations.
- **Optimise resolution of fully bound concludables**
  In some cases, a concludable that is being resolved may have all of its retrievable variables (named and anonymous) fully bound. This means that only a single answer is required to prove the concludable holds, rather than all possible ways. We add short circuiting to prevent exploring further rules in a concludable when a single answer has been found.

Install & Run: http://docs.vaticle.com/docs/running-typedb/install-and-run

## New Features
${features.map { feature -> createCommitNote(feature) }.joinToString("\n")}
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski could use method reference here:

Suggested change
${features.map { feature -> createCommitNote(feature) }.joinToString("\n")}
${features.map(::createCommitNote).joinToString("\n")}

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in fa97b42.

${features.map { feature -> createCommitNote(feature) }.joinToString("\n")}

## Bugs Fixed
${bugs.map { bug -> createCommitNote(bug) }.joinToString("\n")}
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski could use method reference here:

Suggested change
${bugs.map { bug -> createCommitNote(bug) }.joinToString("\n")}
${bugs.map(::createCommitNote).joinToString("\n")}

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in fa97b42.

${bugs.map { bug -> createCommitNote(bug) }.joinToString("\n")}

## Code Refactor
${refactors.map { refactor -> createCommitNote(refactor) }.joinToString("\n")}
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski could use method reference here:

Suggested change
${refactors.map { refactor -> createCommitNote(refactor) }.joinToString("\n")}
${refactors.map(::createCommitNote).joinToString("\n")}

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in fa97b42.

${refactors.map { refactor -> createCommitNote(refactor) }.joinToString("\n")}

## Other Improvements
${others.map { other -> createCommitNote(other) }.joinToString("\n")}
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski could use method reference here:

Suggested change
${others.map { other -> createCommitNote(other) }.joinToString("\n")}
${others.map(::createCommitNote).joinToString("\n")}

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in fa97b42.

val releaseNote = """
Install & Run: http://docs.vaticle.com/docs/running-typedb/install-and-run

## New Features
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski code below could be generified with something like

val compileNotes = { notes: Iterable<CommitNote> -> notes.map(::createCommitNote).joinToString("\n") }

and then in the string e.g.:

## Bugs Fixed
${compileNotes(bugs)}

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm I think we should keep the current code, because your suggestion is too terse.

.setHeaders(HttpHeaders().setAuthorization("Token $githubToken").setAccept("application/vnd.github.v3+json"))
.execute()
val body = Json.parse(String(response.content.readBytes()))
return body.asObject().get("commits").asArray().map { e -> e.asObject().get("sha").asString() }
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski Kotlin allows but doesn't require naming lambda arguments:

Suggested change
return body.asObject().get("commits").asArray().map { e -> e.asObject().get("sha").asString() }
return body.asObject().get("commits").asArray().map { it.asObject().get("sha").asString() }

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I know of this, but it felt too magical and I am reluctant to use it.

@lolski lolski merged commit 1a10c8d into typedb:master Nov 17, 2021
@lolski lolski deleted the release-notes branch November 17, 2021 19:44

private fun getEnv(env: String): String {
return System.getenv(env) ?: throw RuntimeException("'$env' environment variable must be set.")
}
Copy link
Member Author

Choose a reason for hiding this comment

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

newline

import java.nio.file.Paths
import kotlin.io.path.notExists

fun main(args: Array<String>) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the entry point of the program. Given a repo, version, commit and a template file:

  • it will collect all the commits between the commit that is to be released to the last released commit
  • for each collected commit, write the title and description into the release note

import com.vaticle.dependencies.tool.release.createnotes.Constant.github
import java.nio.file.Path

fun getCommits(org: String, repo: String, current: Version, to: String, baseDir: Path, githubToken: String): List<String> {
Copy link
Member Author

Choose a reason for hiding this comment

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

This method collects all the commits between the commit that is to be released to the commit that belongs to the immediately preceding release.

If there is no preceding release found, that means this commit is the very first release, and all commits down to the first commit will be collected.

enum class Type { FEATURE, BUG, REFACTOR, OTHER }
}

fun getCommitDescriptions(org: String, repo: String, commits: List<String>, githubToken: String): List<CommitDescription> {
Copy link
Member Author

Choose a reason for hiding this comment

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

this function collects the commit description from the commit:

  • if the commit is created from a PR, the title and description will be used.
  • if the commit is directly pushed, the first line will be collected and the rest discarded. commit message may be less structured than a PR message and we do not want to accidentally add implementation detail.

Copy link
Member Author

Choose a reason for hiding this comment

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

now that I think about it, it's better to filter out the "implementation" section the PR description here

import java.nio.file.Path
import kotlin.text.Charsets.UTF_8

fun createCommitNoteMd(description: CommitDescription): String {
Copy link
Member Author

Choose a reason for hiding this comment

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

make private

return "- **${description.title}**\n $desc"
}

fun writeReleaseNoteMd(commitDescriptions: List<CommitDescription>, releaseTemplateFile: Path) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Write the release notes:

  1. Create the markdown string of each CommitDescription into the right section of the markdown string, ie., whether it's a feature, bug, refactor, or "other".
  2. Write the markdown string into the file.


package com.vaticle.dependencies.tool.release.createnotes

data class Version(val major: Int, val minor: Int, val patch: Int, val alpha: Int?): Comparable<Version> {
Copy link
Member Author

Choose a reason for hiding this comment

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

This class represents our semantic version, such as 2.0.0, 2.0.0-alpha, and 2.0.0-alpha-3.

It implements Comparable because we need it to be sortable.

patch = version2[2].toInt(),
alpha = null
)
} else if (version1.size == 2) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This else if branch handles when we get a string such as 2.0.0-alpha. It implies that this is the first alpha and hence set alpha = 1 when instantiating the Version class.

fun createCommitNoteMd(description: CommitDescription): String {
val desc = StringBuilder()
var header = 0
for (line in description.desc.lines()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@lolski can make the implementation functional:

val desc = description.desc.lines().reversed().takeWhile { !it.startsWith("## ") }.reversed()

alexjpwalker pushed a commit to alexjpwalker/dependencies that referenced this pull request Nov 23, 2021
## What is the goal of this PR?

We've updated the release notes script to collect from commits rather than milestones. It will collect the commits between the last release up to the specified commit being released.

Additionally, it's been rewritten in Kotlin.

## What are the changes implemented in this PR?

1. Update the script for collecting release note from commits, which works as follows:
    - given C1, the commit that we want to release, collect L, the list of commits that we want to add to the release note:
      - if this is the first release ever, L would contain all the commits from C1 down to and including the first commit in the repo
      - if this is not the first release:
        - find C2, the commit of the release that immediately precedes this release
        - L would contain all the commits from C1 down to but excluding C2
    - write PR description or commit message for each commit in L into the release note
2. Rewrite in Kotlin
lolski pushed a commit to lolski/dependencies that referenced this pull request Oct 7, 2022
## What is the goal of this PR?

Allow assembling and deploying Crate packages

## What are the changes implemented in this PR?

Implement `assemble_crate` and `deploy_crate` rules

Fixes typedb#319
lolski pushed a commit to lolski/dependencies that referenced this pull request Oct 7, 2022
## What is the goal of this PR?

Initial implementation of `assemble_crate` and `deploy_crate` done in typedb#322 is not fully compatible with `crates.io` registry and therefore needs to be made compatible.

## What are the changes implemented in this PR?

* Make `description`, `license` and `repository` fields mandatory
* Correctly handle empty `documentation`
* Validate `homepage`, `repository` and `documentation` fields to be valid URLs
* Validate `keywords` list to be valid
* Account for possibility of validation errors on deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants