bitflicker64 commented on code in PR #360: URL: https://github.com/apache/hugegraph-computer/pull/360#discussion_r3969920252
########## .github/workflows/release-notes.yml: ########## @@ -0,0 +1,42 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +name: "Release Notes Generator" + +on: + push: + tags: + - '*.*.*' Review Comment: ⚠️ important: this tag filter also fires on release-candidate tags, so every vote round would leave a stray draft release behind. GitHub tag filters are globs, not regular expressions, and `*` matches any character except `/`. `*.*.*` therefore matches `1.5.0-rc1`, splitting it as `1` + `.` + `5` + `.` + `0-rc1`. Four of the eight tags this repository has are RC tags (`1.0.0-rc2`, `1.5.0-rc1`, `1.5.0-rc2`, `1.5.0-rc3`, per `git ls-remote --tags origin`), so cutting 1.5.0 would have drafted four releases instead of one. `draft: true` means nothing is published, but each one has to be deleted by hand during the vote. This is the other side of the `v*` mismatch @imbajin raised on this line: the new pattern does reach the real tags, and now reaches more than them. Character ranges and `+` are supported in these filters, so the release form can be matched exactly: ```yaml - '[0-9]+.[0-9]+.[0-9]+' ``` If you change it, `docs/automation-guide.md:58` hardcodes `*.*.*` and needs the same edit. ########## .github/workflows/release-notes.yml: ########## @@ -0,0 +1,42 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +name: "Release Notes Generator" + +on: + push: + tags: + - '*.*.*' + +jobs: + generate-release-notes: + name: Generate Release Draft & Notes + runs-on: ubuntu-latest + permissions: + contents: write + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Generate Release Notes + uses: softprops/action-gh-release@v2 Review Comment: ⚠️ important: this action is referenced by a tag the project does not control, and this job hands it a write token. Lines 28-29 grant `contents: write`, and `softprops/action-gh-release@v2` is a moving major tag on a third-party repository. Whoever can move that tag decides what runs with that token, at the moment a release is being cut. `commit-check.yml:39` has the same shape with `amannn/action-semantic-pull-request@v5`, contained by `pull-requests: read`. This is the first workflow here to grant a third-party action `contents: write` explicitly. I am not claiming it is the first to hold one: `computer-ci.yml` and `vermeer-ci.yml` declare no `permissions:` block at all, so `codecov/codecov-action@v3` and the two `docker/*` actions run under the repository default, and `gh api repos/apache/hugegraph-computer/actions/permissions/workflow` answers 403 for me, so I cannot read what that default is. Nothing in the tree is SHA-pinned today, so this is not a regression against house style. Please pin both new actions to a full commit SHA and keep the readable version in a trailing comment: ```yaml uses: softprops/action-gh-release@<40-char-sha> # v2.x.y ``` ########## docs/automation-guide.md: ########## @@ -0,0 +1,72 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> + +# Automation & Code Quality Guide + +This guide details the repository automations and code quality tools configured for **HugeGraph Computer** and **Vermeer** as recommended in Automation Analysis [#320](https://github.com/apache/hugegraph-computer/issues/320). + +--- + +## Code Quality & Formatting Automations + +### 1. Code Style & License Checks +- **Checkstyle (`maven-checkstyle-plugin`):** Enforces Java coding style guidelines defined in `checkstyle.xml`. +- **Apache RAT (`apache-rat-plugin`):** Verifies Apache license headers across all project source files. + +```bash +# Run Checkstyle validation +mvn checkstyle:check + +# Run Apache RAT license validation +mvn apache-rat:check +``` + +### 2. Test Coverage (`jacoco-maven-plugin`) +JaCoCo tracks unit and integration test coverage during build execution. + +```bash +# Run unit tests and generate JaCoCo coverage report +mvn test -P unit-test + +# Inspect generated report at: +# target/site/jacoco/jacoco.xml +``` + +--- + +## CI/CD Workflows + +| Workflow | Path | Trigger | Description | +|----------|------|---------|-------------| +| **Commit Check** | `.github/workflows/commit-check.yml` | Pull Request | Validates PR titles against Conventional Commits formatting rules. | +| **Computer CI** | `.github/workflows/computer-ci.yml` | Push / PR | Compiles, runs RAT, HDFS, K8s, and Java unit/integration tests. | +| **Vermeer CI** | `.github/workflows/vermeer-ci.yml` | Push / PR | Builds Vermeer Go binary, checks UI assets, and tests Docker builds. | +| **Release Notes** | `.github/workflows/release-notes.yml` | Tag Push (`*.*.*`) | Automatically drafts GitHub release notes from git history. | Review Comment: 🧹 minor: two things in this row will mislead a reader. `Tag Push (`*.*.*`)` copies the trigger pattern into prose, so it goes stale as soon as the pattern is corrected, which the comment on `release-notes.yml:22` asks for. Point at the workflow's `on:` block rather than duplicating the glob. "from git history" is not what the workflow does. `generate_release_notes: true` is handled by the GitHub releases API, which assembles the notes from merged pull requests and commits between tags on the server side. See the comment on `release-notes.yml:31`. Line 55 has a smaller version of the same problem: it calls the workflow "Commit Check", while `commit-check.yml:17` names it `Commit & PR Title Validation` and its job at line 33 is `Validate PR Title & Format`. A table meant as a map should use the names that show up on the checks tab. ########## docs/automation-guide.md: ########## @@ -0,0 +1,72 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> + +# Automation & Code Quality Guide + +This guide details the repository automations and code quality tools configured for **HugeGraph Computer** and **Vermeer** as recommended in Automation Analysis [#320](https://github.com/apache/hugegraph-computer/issues/320). + +--- + +## Code Quality & Formatting Automations + +### 1. Code Style & License Checks +- **Checkstyle (`maven-checkstyle-plugin`):** Enforces Java coding style guidelines defined in `checkstyle.xml`. +- **Apache RAT (`apache-rat-plugin`):** Verifies Apache license headers across all project source files. + +```bash +# Run Checkstyle validation +mvn checkstyle:check Review Comment: 🧹 minor: these commands fail as written, because there is no `pom.xml` at the repository root. The Maven reactor root is `computer/pom.xml`, which is why `computer-ci.yml:13-15` sets `defaults.run.working-directory: computer`. Run from the repository root, `mvn checkstyle:check`, `mvn apache-rat:check` and `mvn test -P unit-test` (line 43) all stop on "there is no POM in this directory". The report path at line 46 has the same dependency. `computer/computer-test/pom.xml` binds `report-aggregate` with `<outputDirectory>${basedir}/../target/site/jacoco</outputDirectory>`, so `target/site/jacoco/jacoco.xml` resolves only relative to `computer/`. Please state the working directory once at the top of this section, or make the commands self-locating: ```bash mvn -f computer/pom.xml checkstyle:check ``` ########## .github/workflows/release-notes.yml: ########## @@ -0,0 +1,42 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +name: "Release Notes Generator" + +on: + push: + tags: + - '*.*.*' + +jobs: + generate-release-notes: + name: Generate Release Draft & Notes + runs-on: ubuntu-latest + permissions: + contents: write + steps: + - name: Checkout repository Review Comment: 🧹 minor: nothing in this job reads the checked-out tree, so this step and its full-history clone can go. The only other step is `softprops/action-gh-release` with `generate_release_notes: true` and no `files:` or `body_path:` input. In that configuration the action passes `generate_release_notes` through to the GitHub releases API, which builds the notes server-side from the tag range; it does not read the working directory. `fetch-depth: 0` then clones the entire history of the repository on every tag push with no consumer. Please drop the checkout step, or at minimum `fetch-depth: 0` on line 34. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
