Next Gen GitHub Tag Action #252

beholdenkey · 2023-04-03T12:50:14Z

Summary of changes

This is a first iteration refactor. A lot of the code has been modified and updated. However, there is still a lot of room for improvement. I have a dev branch where I am testing out switching from the node server to using Python for the GitPython and SemVer Libraries. That will offer a greater level of performance and extensibility.

In addition, there is now a publish.yml that will build and publish the container image to the GitHub container registry. I saw this was something brought up in previous pull requests. I still need to tie the action into using that image, but it should help with the action time consumed.

Breaking Changes

There are not necessarily breaking changes. The functionality of the action has stayed the same; however,r a lot of the code supporting it has.

How changes have been tested

Local Unit Tests, Testing the container build/workflows; this will be the first iteration of the pull request to the master origin.

List any unknowns

Lots of changes/lots of unknowns. This will more than likely require iterations to complete changes and successfully merge.

Updated Code of Conduct

Tag check contains the tests that used to be in the test.yml

Updated to be more concise

matthiasgubler · 2023-04-04T13:54:46Z

action.yml

+inputs:
+  DEFAULT_BUMP:
+    description: "Default semver bump type (major, minor, patch, or none)"
+    required: false
+    default: "minor"
+  DEFAULT_BRANCH:
+    description: "The default branch for the repository"
+    required: false


Question: why are you adding these two values as inputs when env-vars provide everything else? Keeping this way would mean, to adjust the examples to something like:

- name: Bump version and push tag uses: anothrNick/[email protected] # Don't use @master unless you're happy to test the latest version env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} WITH_V: true with: DEFAULT_BUMP: "patch"

As the inputs need to be provided in a different matter. Also accessing the values would need to change to:

${{ inputs.DEFAULT_BUMP }}

Last but not least, they should be renamed to kebab-case default-bump.

I would delete lines 5 - 12 in this case

sbe-arg

Thank you for this. On the phone it looks a considerable improvement in practice.

I have 0 time to check this pr this week but maybe @sammcj can have a look and test it.

sammcj

Awesome effort! Really appreciate the contributions.

One thing I'd say especially given this is a big PR - it should really include some basic tests.

It doesn't have to be some big fancy testing framework but at least a script or functions that validate several scenarios that this may be used for.

sammcj · 2023-04-07T22:17:40Z

.github/workflows/publish.yml

+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+      - name: Install cosign


Oh nice! Great to see some security improvements!

sammcj · 2023-04-07T22:18:14Z

.github/workflows/scripts/tag_check.sh

@@ -0,0 +1,45 @@
+#!/bin/bash


!/usr/bin/env bash

Just so it's portable.

sammcj · 2023-04-07T22:28:33Z

.github/workflows/scripts/tag_check.sh

@@ -0,0 +1,45 @@
+#!/bin/bash
+
+set -euo pipefail


A nice little addition I like to add to shell scripts to add set -x only if you're debugging:

DEBUG=${DEBUG:-false} function debug() { echo "Debug mode enabled" >>"$GITHUB_STEP_SUMMARY" # Print all environment variables env | sort >>"$GITHUB_STEP_SUMMARY" echo -e "\n" >>"$GITHUB_STEP_SUMMARY" set -x } if [[ $DEBUG == 'true' ]]; then debug fi

sammcj · 2023-04-07T22:34:44Z

.github/workflows/scripts/tag_check.sh

+echo "MAIN Part: $MAIN_OUTPUT_PART" >>"$GITHUB_STEP_SUMMARY"
+echo "PRE Tag: $PRE_OUTPUT_TAG" >>"$GITHUB_STEP_SUMMARY"
+echo "PRE New tag: $PRE_OUTPUT_NEWTAG" >>"$GITHUB_STEP_SUMMARY"
+echo "PRE Part: $PRE_OUTPUT_PART" >>"$GITHUB_STEP_SUMMARY"


A little trick with this to make it render in a nice table can be something like this:

OUTPUT_SUMMARY="\n | Output | Value | \n | ------------ | --------------------- | \n | | | \n | MAIN Tag | ${MAIN_OUTPUT_TAG} | \n | MAIN New tag | ${MAIN_OUTPUT_NEWTAG} | \n | MAIN Part | ${MAIN_OUTPUT_PART} | \n | PRE Tag | ${PRE_OUTPUT_TAG} | \n | PRE New tag | ${PRE_OUTPUT_NEWTAG} | \n | PRE Part | ${PRE_OUTPUT_PART} | \n " echo -e "$OUTPUT_SUMMARY" >>"$GITHUB_STEP_SUMMARY"

sammcj · 2023-04-07T22:35:38Z

CODE_OF_CONDUCT.md


-Examples of unacceptable behaviour include:
+Examples of unacceptable behavior include:


Behaviour is the correct international spelling.

Behavior is specific to American.

sammcj · 2023-04-07T22:38:58Z

.github/workflows/scripts/tag_check.sh

+
+# Test for #none bump
+if [[ $MAIN_OUTPUT_PART == "none" ]]; then
+	if [[ $MAIN_OUTPUT_TAG == "$MAIN_OUTPUT_NEWTAG" ]]; then


I think the variable on the left of the comparison should be quoted as well.

sammcj · 2023-04-07T22:40:25Z

.github/workflows/scripts/tag_check.sh

+}
+
+main="$(verlt "$MAIN_OUTPUT_TAG" "$MAIN_OUTPUT_NEWTAG" && true || false)"
+pre="$(verlt "$PRE_OUTPUT_TAG" "$PRE_OUTPUT_NEWTAG" && true || false)"


It feels weird to have double quotes inside a double quoted string but it does seem to work on my machine as well 🤷

sbe-arg · 2023-05-08T12:44:04Z

entrypoint.sh

-# set outputs
-setOutput "new_tag" "$new"
-setOutput "part" "$part"
-setOutput "tag" "$new" # this needs to go in v2 is breaking change


I would like this comment to remain specially since broke the flows for a LOT lots of people

sbe-arg · 2023-05-26T00:49:46Z

:(

beholdenkey added 12 commits April 3, 2023 07:17

applied formatting and other fixes

221b1b5

Update CODE_OF_CONDUCT.md

57c2963

Updated Code of Conduct

Update entrypoint.sh

8ebc2ab

Update Dockerfile

8ee5792

Update action.yml

ed87534

Update pull_request_template.md

fd09583

Create tag_check.sh

7369c9a

Tag check contains the tests that used to be in the test.yml

Update lint.yml

34c6984

Update main.yml

f368c74

Update test.yml

237a5ff

Update CONTRIBUTING.md

92ea001

Updated to be more concise

Update README.md

19d6f3e

beholdenkey mentioned this pull request Apr 3, 2023

📣 LOOKING FOR MAINTAINERS 📣 #238

Open

beholdenkey added 8 commits April 3, 2023 08:52

Create publish.yml

9d5c351

Update publish.yml

b2f1d5a

Update action.yml

4b84757

Update publish.yml

f2525ab

Update action.yml

c00c475

Update entrypoint.sh

441a86c

Update tag_check.sh

6b18d80

Update pull_request_template.md

387b939

matthiasgubler reviewed Apr 4, 2023

View reviewed changes

sbe-arg reviewed Apr 7, 2023

View reviewed changes

sammcj requested changes Apr 7, 2023

View reviewed changes

sbe-arg requested changes May 8, 2023

View reviewed changes

beholdenkey closed this by deleting the head repository May 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Next Gen GitHub Tag Action #252

Next Gen GitHub Tag Action #252

beholdenkey commented Apr 3, 2023 •

edited

Loading

matthiasgubler Apr 4, 2023

sbe-arg left a comment

sammcj left a comment

sammcj Apr 7, 2023

sammcj Apr 7, 2023

sammcj Apr 7, 2023

sammcj Apr 7, 2023

sammcj Apr 7, 2023

sammcj Apr 7, 2023

sammcj Apr 7, 2023

sbe-arg May 8, 2023

sbe-arg commented May 26, 2023


		Examples of unacceptable behaviour include:
		Examples of unacceptable behavior include:

Next Gen GitHub Tag Action #252

Next Gen GitHub Tag Action #252

Conversation

beholdenkey commented Apr 3, 2023 • edited Loading

Summary of changes

Breaking Changes

How changes have been tested

List any unknowns

Choose a reason for hiding this comment

sbe-arg left a comment

Choose a reason for hiding this comment

sammcj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbe-arg commented May 26, 2023

beholdenkey commented Apr 3, 2023 •

edited

Loading