bitflicker64 commented on code in PR #360:
URL:
https://github.com/apache/hugegraph-computer/pull/360#discussion_r3901065756
##########
computer/pom.xml:
##########
@@ -431,6 +431,36 @@
</execution>
</executions>
</plugin>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>2.30.0</version>
+ <configuration>
+ <java>
+ <removeUnusedImports />
+ </java>
+ </configuration>
+ </plugin>
+ <plugin>
Review Comment:
⚠️ important — This duplicates JaCoCo, which is already configured in
`computer/computer-test/pom.xml:176-198` (v0.8.4, `pre-test`/`prepare-agent` +
`post-test`/`report-aggregate` writing to `${basedir}/../target/site/jacoco`,
the exact path `computer-ci.yml:110-115` uploads to Codecov). A parent-level
`<build><plugins>` entry is inherited by every module and merges with the child
declaration instead of replacing it.
`mvn -o help:effective-pom` at this head:
- `-pl computer-test` → version resolves to **0.8.4** (the child's explicit
version wins, so the declared 0.8.8 never takes effect here) with **four**
executions: the new `prepare-agent` and `report` plus the existing `pre-test`
and `post-test`.
- `-pl computer-core` → version **0.8.8**.
So sibling modules instrument with the 0.8.8 agent while `computer-test`'s
0.8.4 `report-aggregate` analyzes those `jacoco.exec` files, and
`computer-test` runs a redundant `report` alongside `report-aggregate` in the
same `test` phase. (The duplicated `prepare-agent` itself is harmless — the
second execution overwrites `argLine` with an identical value rather than
adding a second `-javaagent`.)
Please drop this block, or consolidate to a single declaration: remove the
`computer-test` one, settle on one version, and keep the `report-aggregate`
output path that Codecov consumes.
##########
computer/pom.xml:
##########
@@ -431,6 +431,36 @@
</execution>
</executions>
</plugin>
+ <plugin>
Review Comment:
🧹 minor — The only step configured here is `<removeUnusedImports/>`, which
duplicates a rule the build already enforces: `checkstyle.xml:47,49` declares
`RedundantImport` and `UnusedImports`, and `maven-checkstyle-plugin` binds
`check` to the `validate` phase with `failsOnError=true`
(`computer/pom.xml:373-394`).
There is also no `<executions>` binding and `computer-ci.yml` was not
updated to call `spotless:check`, so nothing runs automatically despite the PR
describing "Automatic Code Formatting" — only a manual `mvn spotless:apply`.
For what it's worth the plugin does work and the tree is already clean: `mvn -B
spotless:check` passes across all 10 modules at this head.
Either configure steps Checkstyle does not already cover (import order,
license header, a formatter) and wire `spotless:check` into a phase and CI, or
drop the plugin along with its `docs/automation-guide.md` section — as
configured it adds a build dependency for no net capability.
##########
.github/workflows/release-notes.yml:
##########
@@ -0,0 +1,40 @@
+#
+# 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:
+ - 'v*'
+
+jobs:
+ generate-release-notes:
+ name: Generate Release Draft & Notes
+ runs-on: ubuntu-latest
Review Comment:
⚠️ important — This job declares no `permissions:` block, but
`softprops/action-gh-release` documents `permissions: contents: write` as
required to create a release. With no block the token falls back to the
repository/org default; where that default is read-only the step fails with
HTTP 403 at tag time, i.e. exactly when a release is being cut. This is
separate from the tag-pattern issue already raised below: correcting the
trigger alone still leaves the job unable to create the draft.
It also diverges from this repo's convention of explicit least privilege —
`stale.yml:11-13` (`issues: write`, `pull-requests: write`),
`codeql-analysis.yml:23-26` (`actions: read`, `contents: read`,
`security-events: write`), `rerun-ci.yml:10` (`permissions: {}`).
Please add to this job:
```yaml
permissions:
contents: write
```
(I could not read the repository's effective default workflow permission —
the API returns 403 for my token — but declaring it explicitly is correct
either way.)
##########
.github/workflows/commit-check.yml:
##########
@@ -0,0 +1,52 @@
+#
+# 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: "Commit & PR Title Validation"
+
+on:
+ pull_request:
+ types:
+ - opened
+ - edited
+ - synchronize
+ - reopened
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
+
+jobs:
+ validate-pr-title:
+ name: Validate PR Title & Format
+ runs-on: ubuntu-latest
Review Comment:
🧹 minor — This job also declares no `permissions:` block, so it inherits the
default token scope. It only needs to read the PR title, and the rest of this
repo pins least privilege explicitly (`stale.yml:11-13`,
`codeql-analysis.yml:23-26`, `rerun-ci.yml:10`).
Please add:
```yaml
permissions:
pull-requests: read
```
--
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]
