git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Matthias Aßhauer" <mha1993@live.de>
To: Mahdi Hosseinzadeh via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
	Mahdi Hosseinzadeh <mdihosseinzadeh@gmail.com>,
	johannes.schindelin@gmx.de
Subject: Re: [PATCH] githubci: add a workflow for creating GitHub release notes
Date: Thu, 25 Nov 2021 21:57:13 +0100 (CET)	[thread overview]
Message-ID: <AM0PR04MB60196EFE984652ECCBD591A8A5629@AM0PR04MB6019.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <pull.1146.git.git.1637840216877.gitgitgadget@gmail.com>


Hi Mahdi,

I've already written up these concerns on GitHub [1] and you've replied 
there, but Johannes asked me to also send this to the mailing list, so 
please bear with me for mostly repeating the same points.

On Thu, 25 Nov 2021, Mahdi Hosseinzadeh via GitGitGadget wrote:

> From: Mahdi Hosseinzadeh <mdihosseinzadeh@gmail.com>
>
> GitHub now allows users to subscribe only to
> "release" notifications of a repository.
> So, users can be notified of new releases and their
> changelog/release notes automatically.
>
> This workflow works whenever:
>    a new version tag
>    (with the format following the regex "v\d+\..*")
>    is pushed to the repository
> AND
>    the commit that the tag points to, created/modified
>    a release notes file from Doumentation/RelNotes/ directory.
>
> The script for generating the temporary changelog file is
> written in Kotlin language which can be a much better alternative
> to shell scripts in terms of features and readability
> (it is like a python script but with static typing support).
> The Kotlin runner is pre-installed in GitHub Actions environments;
> for more information see
>    https://github.com/actions/virtual-environments/
>    https://stackoverflow.com/a/69116750/8583692
>
> The "Release Notes (yyyy-MM-dd)" link in https://git-scm.com/
> website can also link to these GitHub release pages instead of
> to the raw .txt release note file in the repository.
>
> See the issue related to GitHub release notifications:
> https://github.com/isaacs/github/issues/410
>
> Also see GitHub announcement for this feature:
> https://github.blog/changelog/2018-11-27-watch-releases/

Nit: "Github now allows users" sounds like a new feature, not one that's 
three years old.

>
> Signed-off-by: Mahdi Hosseinzadeh <mdihosseinzadeh@gmail.com>
> ---
>    Add a workflow for creating GitHub release notes
>
>    Because this is not directly the git code and is related to GitHub CI, I
>    post it here.
>
>    This pull request adds a new GitHub Actions workflow that automatically
>    creates releases on GitHub repository when pushing a new tag to the
>    repository.
>
>    GitHub now allows users to subscribe only to "release" notifications of
>    a repository. So, users can be notified of new releases and their
>    changelog/release notes automatically.
>
>    This workflow works whenever: a new version tag (with the format
>    following the regex v\d+\..*) is pushed to the repository AND the commit
>    that the tag points to, created/modified a release notes file from
>    Doumentation/RelNotes/ directory.
>
>    The script for generating the temporary changelog file is written in
>    Kotlin language [https://kotlinlang.org/] which can be a better
>    alternative to shell scripts in terms of features and readability (it is
>    like a python script but with static typing support). The Kotlin runner
>    is pre-installed in GitHub Actions environments; for more information
>    see https://github.com/actions/virtual-environments/
>    https://stackoverflow.com/a/69116750/8583692
>
>    The Release Notes (yyyy-MM-dd) link in https://git-scm.com/ website can
>    also link to these GitHub release pages instead of to the raw .txt
>    release note file in the repository.
>
>    See the issue related to GitHub release notifications:
>    https://github.com/isaacs/github/issues/410
>
>    Also see GitHub announcement for this feature:
>    https://github.blog/changelog/2018-11-27-watch-releases/
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1146%2Fmahozad%2Fadd-github-releases-workflow-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1146/mahozad/add-github-releases-workflow-v1
> Pull-Request: https://github.com/git/git/pull/1146
>
> .../generate-github-changelog.main.kts        | 21 ++++++++++
> .github/workflows/create-release.yml          | 40 +++++++++++++++++++
> 2 files changed, 61 insertions(+)
> create mode 100644 .github/scripts/generate-github-changelog.main.kts
> create mode 100644 .github/workflows/create-release.yml
>
> diff --git a/.github/scripts/generate-github-changelog.main.kts b/.github/scripts/generate-github-changelog.main.kts
> new file mode 100644
> index 00000000000..e57fd2a6ae5
> --- /dev/null
> +++ b/.github/scripts/generate-github-changelog.main.kts
> @@ -0,0 +1,21 @@
> +#!/usr/bin/env kotlin
> +
> +/**
> + * Copies contents of the release notes file created/modified
> + * in this commit to a new file to be used by the workflow.
> + */
> +
> +import java.io.File
> +
> +println("Files modified in this commit:")
> +args.forEachIndexed { index, name ->
> +    println("\t${index + 1}- $name")
> +}
> +
> +val notesFile = args
> +    .map(::File)
> +    .singleOrNull { "RelNotes" in it.parent }
> +
> +notesFile
> +    ?.copyTo(File("changelog.txt"))
> +    ?: println("No release notes file modified in this commit")

We need to spin up a JVM for 21 lines of code just to copy a single 
file. I think a single call of `cp` is faster and more readable than that.

> diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml
> new file mode 100644
> index 00000000000..711ba105e42
> --- /dev/null
> +++ b/.github/workflows/create-release.yml
> @@ -0,0 +1,40 @@
> +name: Create GH release
> +
> +# Create a GitHub release for each new tag.
> +# The release notes are taken from the release notes file
> +# modified in that commit located in Documentation/RelNotes directory.
> +
> +on:
> +  push:
> +    tags:
> +      - v[0-9]+.*

I think we should probably exclude the release candidates from this. As 
Johhannes pointed out[2], marking them as full releases would 
periodically cause https://github.com/git/git/releases/latest to point
to a pre-release instead of the latest full release.

> +
> +permissions:
> +  contents: write
> +
> +jobs:
> +  create-gh-release:
> +    name: Create a new release or update an existing release in the GitHub repository
> +    runs-on: ubuntu-latest
> +    steps:
> +      - name: Checkout the repository
> +        uses: actions/checkout@v2
> +        with:
> +          fetch-depth: 2  # OR '0' To retrieve all preceding commit.

The value 2 seems pretty arbitrary and the comment adds nothing.

> +      - name: Get changed files
> +        uses: tj-actions/changed-files@v11.7
> +        id: changed-files

You've replied on Github that you need the last two commits for this 
action [3], but I don't think we care about wether or not the release 
notes where changed in the last commit. We only need the version number 
(from the pushed tag) to determine the correct release notes file.

> +        with:
> +          separator: ' '
> +      - name: Generate the changelog
> +        run: kotlin .github/scripts/generate-github-changelog.main.kts ${{ steps.changed-files.outputs.all_changed_and_modified_files }}
> +      - name: Create the release
> +        uses: actions/create-release@v1
> +        env:
> +          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
> +        with:
> +          tag_name: ${{ github.ref_name }}
> +          release_name: ${{ github.ref_name }}
> +          body_path: changelog.txt

If we just use the file directly we don't even need to copy the file to 
changelog.txt

> +          draft: false
> +          prerelease: false

If we don't exclude release candidates, we should set prelease to true for 
those tags.

>
> base-commit: 5f439a0ecfb4657100ec1e56ef9c6eca963b5a94
> -- 
> gitgitgadget
>

All in all I think this is too convoluted for what it's trying to 
achieve. I think we should be able to achieve the same result with 
something like this:

  .github/workflows/create-release.yml | 37 ++++++++++++++++++++++++++++
  1 file changed, 37 insertions(+)
  create mode 100644 .github/workflows/create-release.yml

diff --git a/.github/workflows/create-release.yml 
b/.github/workflows/create-release.yml
new file mode 100644
index 0000000000..5b9fdf0372
--- /dev/null
+++ b/.github/workflows/create-release.yml
@@ -0,0 +1,37 @@
+name: Create GH release
+
+# Create a GitHub release for each new tag.
+# The release notes are taken from the release notes file
+# modified in that commit located in Documentation/RelNotes directory.
+
+on:
+  push:
+    tags:
+      - v[0-9]+.[0-9]+.[0-9]+
+
+permissions:
+  contents: write
+
+jobs:
+  create-gh-release:
+    name: Create a new release or update an existing release in the 
GitHub repository
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v2
+        with:
+          fetch-depth: 1
+      - name: Get version number
+        shell: bash
+        run: |
+          echo GIT_VERSION=${GITHUB_REF#refs/tags/v} >> $GITHUB_ENV
+      - name: Create the release
+        uses: actions/create-release@v1
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        with:
+          tag_name: ${{ github.ref_name }}
+          release_name: ${{ github.ref_name }}
+          body_path: Documentation/RelNotes/${{ env.GIT_VERSION }}.txt
+          draft: false
+          prerelease: false
-- 
2.25.1

An example of the result this reduced action produces can be found at [4] 
(release notes for v2.34.1, but the tagged commit isn't v2.34.1).

Best regards

Matthias

[1] https://github.com/git/git/pull/1146
[2] https://github.com/git/git/pull/1146#discussion_r756854259
[3] https://github.com/git/git/pull/1146#discussion_r756845042
[4] https://github.com/rimrul/git/releases/tag/v2.34.1

  reply	other threads:[~2021-11-25 20:59 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-25 11:36 [PATCH] githubci: add a workflow for creating GitHub release notes Mahdi Hosseinzadeh via GitGitGadget
2021-11-25 20:57 ` Matthias Aßhauer [this message]
2021-11-26 13:59   ` Johannes Schindelin
2021-11-26 17:37     ` Matthias Aßhauer
2021-11-29 12:49       ` Ævar Arnfjörð Bjarmason
2021-11-30 11:46         ` Johannes Schindelin
2021-12-02 19:05           ` Junio C Hamano
2021-12-03  8:33             ` Fabian Stelzer
2021-12-05  1:25               ` Junio C Hamano
2021-12-05 10:54                 ` Fabian Stelzer
2021-12-07  0:05                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR04MB60196EFE984652ECCBD591A8A5629@AM0PR04MB6019.eurprd04.prod.outlook.com \
    --to=mha1993@live.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=mdihosseinzadeh@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).