Skip to content

Commit 3879412

Browse files
authored
chore: address follow-up review feedback from PRs 18, 20, and 21 (#22)
* Clarify Docker port wording in contributing guide * Guard SQLite sequence resets in tests * Harden release workflow trigger and tag checks
1 parent ec09e65 commit 3879412

5 files changed

Lines changed: 48 additions & 23 deletions

File tree

.github/workflows/release.yml

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,26 @@
11
name: Release
22

33
on:
4-
pull_request_target:
5-
types: [closed]
4+
push:
5+
branches:
6+
- main
67

78
permissions:
89
contents: write
910

1011
concurrency:
11-
group: release-${{ github.event.pull_request.base.ref }}
12+
group: release-${{ github.ref_name }}
1213
cancel-in-progress: false
1314

1415
jobs:
1516
release:
16-
if: >
17-
github.event.pull_request.merged == true &&
18-
github.event.pull_request.base.ref == 'main'
1917
runs-on: ubuntu-latest
2018

2119
steps:
22-
- name: Check out merged commit
20+
- name: Check out pushed commit
2321
uses: actions/checkout@v4
2422
with:
25-
ref: ${{ github.event.pull_request.merge_commit_sha }}
23+
ref: ${{ github.sha }}
2624
fetch-depth: 0
2725

2826
- name: Fetch tags
@@ -31,13 +29,12 @@ jobs:
3129
- name: Determine CalVer tag
3230
id: calver
3331
env:
34-
MERGE_COMMIT_SHA: ${{ github.event.pull_request.merge_commit_sha }}
35-
MERGED_AT: ${{ github.event.pull_request.merged_at }}
32+
RELEASE_COMMIT_SHA: ${{ github.sha }}
3633
run: |
3734
set -euo pipefail
3835
3936
existing_tag="$(
40-
git tag --points-at "$MERGE_COMMIT_SHA" |
37+
git tag --points-at "$RELEASE_COMMIT_SHA" |
4138
grep -E '^v[0-9]{2}\.[0-9]{2}\.[0-9]{2}\.[0-9]+$' |
4239
sort -V |
4340
tail -n 1 || true
@@ -46,7 +43,8 @@ jobs:
4643
if [ -n "$existing_tag" ]; then
4744
tag="$existing_tag"
4845
else
49-
base="$(date -u -d "$MERGED_AT" +'%y.%m.%d')"
46+
commit_timestamp="$(git show -s --format=%cI "$RELEASE_COMMIT_SHA")"
47+
base="$(date -u -d "$commit_timestamp" +'%y.%m.%d')"
5048
max_suffix=0
5149
5250
while IFS= read -r existing; do
@@ -67,31 +65,36 @@ jobs:
6765
6866
- name: Create and push tag
6967
env:
70-
MERGE_COMMIT_SHA: ${{ github.event.pull_request.merge_commit_sha }}
68+
RELEASE_COMMIT_SHA: ${{ github.sha }}
7169
TAG: ${{ steps.calver.outputs.tag }}
7270
run: |
7371
set -euo pipefail
7472
7573
if git ls-remote --exit-code --tags origin "refs/tags/$TAG" >/dev/null 2>&1; then
76-
remote_sha="$(git rev-list -n 1 "$TAG")"
77-
if [ "$remote_sha" = "$MERGE_COMMIT_SHA" ]; then
74+
remote_refs="$(git ls-remote --tags origin "refs/tags/$TAG" "refs/tags/$TAG^{}")"
75+
remote_sha="$(printf '%s\n' "$remote_refs" | awk '$2 ~ /\^\{\}$/ { print $1; exit }')"
76+
if [ -z "$remote_sha" ]; then
77+
remote_sha="$(printf '%s\n' "$remote_refs" | awk 'NF { print $1; exit }')"
78+
fi
79+
80+
if [ "$remote_sha" = "$RELEASE_COMMIT_SHA" ]; then
7881
echo "Tag $TAG already exists on origin and points to the expected commit"
7982
exit 0
8083
fi
8184
82-
echo "Error: Tag $TAG already exists on origin but points to $remote_sha instead of $MERGE_COMMIT_SHA"
85+
echo "Error: Tag $TAG already exists on origin but points to $remote_sha instead of $RELEASE_COMMIT_SHA"
8386
exit 1
8487
fi
8588
8689
git config user.name "github-actions[bot]"
8790
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
88-
git tag -a "$TAG" "$MERGE_COMMIT_SHA" -m "Release $TAG"
91+
git tag -a "$TAG" "$RELEASE_COMMIT_SHA" -m "Release $TAG"
8992
git push origin "refs/tags/$TAG"
9093
9194
- name: Create GitHub release
9295
env:
9396
GH_TOKEN: ${{ github.token }}
94-
MERGE_COMMIT_SHA: ${{ github.event.pull_request.merge_commit_sha }}
97+
RELEASE_COMMIT_SHA: ${{ github.sha }}
9598
TAG: ${{ steps.calver.outputs.tag }}
9699
run: |
97100
set -euo pipefail
@@ -102,6 +105,6 @@ jobs:
102105
fi
103106
104107
gh release create "$TAG" \
105-
--target "$MERGE_COMMIT_SHA" \
108+
--target "$RELEASE_COMMIT_SHA" \
106109
--generate-notes \
107110
--title "$TAG"

CONTRIBUTING.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ docker-compose up -d
3232

3333
Default local ports:
3434

35-
- MySQL/MariaDB on `127.0.0.1:3306`
36-
- PostgreSQL on `127.0.0.1:5432`
35+
- MySQL/MariaDB on host port `3306`
36+
- PostgreSQL on host port `5432`
3737

3838
## Running checks
3939

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ The repository includes:
181181
- PHPStan static analysis
182182
- PHP-CS-Fixer formatting checks
183183
- GitHub Actions CI for pushes and pull requests
184-
- Automatic `vYY.MM.DD.n` CalVer tags and GitHub releases for merged PRs to `main`
184+
- Automatic `vYY.MM.DD.n` CalVer tags and GitHub releases for pushes to `main`
185185

186186
## Learn more
187187

tests/Model/CategoryTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,21 @@ public function setUp(): void
9494
Freshsauce\Model\Model::execute('TRUNCATE TABLE "categories" RESTART IDENTITY');
9595
} elseif (self::$driverName === 'sqlite') {
9696
Freshsauce\Model\Model::execute('DELETE FROM `categories`');
97+
$this->resetSqliteSequenceIfPresent();
98+
}
99+
}
100+
101+
private function resetSqliteSequenceIfPresent(): void
102+
{
103+
try {
97104
Freshsauce\Model\Model::execute(
98105
'DELETE FROM `' . self::SQLITE_SEQUENCE_TABLE . '` WHERE `name` = ?',
99106
['categories']
100107
);
108+
} catch (\PDOException $e) {
109+
if (strpos($e->getMessage(), 'no such table: ' . self::SQLITE_SEQUENCE_TABLE) === false) {
110+
throw $e;
111+
}
101112
}
102113
}
103114

tests/Model/SqliteModelTest.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,18 @@ public static function setUpBeforeClass(): void
2525
protected function setUp(): void
2626
{
2727
App\Model\SqliteCategory::execute('DELETE FROM `categories`');
28-
App\Model\SqliteCategory::execute('DELETE FROM sqlite_sequence WHERE name = ?', ['categories']);
28+
$this->resetSqliteSequenceIfPresent();
29+
}
30+
31+
private function resetSqliteSequenceIfPresent(): void
32+
{
33+
try {
34+
App\Model\SqliteCategory::execute('DELETE FROM sqlite_sequence WHERE name = ?', ['categories']);
35+
} catch (\PDOException $e) {
36+
if (strpos($e->getMessage(), 'no such table: sqlite_sequence') === false) {
37+
throw $e;
38+
}
39+
}
2940
}
3041

3142
public function testSqliteInsertWithDirtyFields(): void

0 commit comments

Comments
 (0)