Copilot commented on code in PR #12774: URL: https://github.com/apache/cloudstack/pull/12774#discussion_r2909505030
########## .github/actions/setup-env/action.yml: ########## @@ -0,0 +1,58 @@ +# 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: 'Setup CloudStack Environment' +description: 'Sets up JDK (with Maven cache), optionally Python, and optionally APT build dependencies for CloudStack.' + +inputs: + java-version: + description: 'The JDK version to use' + required: false + default: '17' + install-python: + description: 'Whether to install Python 3.10' + required: false + default: 'false' + install-apt-deps: + description: 'Whether to install CloudStack APT build dependencies' + required: false + default: 'false' + +runs: + using: "composite" + steps: + - name: Set up JDK ${{ inputs.java-version }} + uses: actions/setup-java@v4 + with: + java-version: ${{ inputs.java-version }} + distribution: 'temurin' + architecture: x64 + cache: 'maven' Review Comment: The composite action pins `actions/setup-java@v4`, but the workflows it replaces were using `actions/setup-java@v5`. This effectively downgrades the setup action (potentially losing fixes/features) and makes versioning inconsistent with the rest of the workflows. Consider updating this to the same major version you were using previously (v5) to keep behavior consistent. ########## .github/workflows/sonar-check.yml: ########## @@ -16,58 +16,45 @@ # under the License. name: Sonar Quality Check - -on: [pull_request] - permissions: - contents: read # to fetch code (actions/checkout) - pull-requests: write # for sonar to comment on pull-request - + contents: read + pull-requests: write +on: + push: + branches: + - main + pull_request: Review Comment: This workflow now runs on `push` to `main` as well as `pull_request`, but it always requests `pull-requests: write`. That permission is only needed for PR decoration/comments and is unnecessary for `push` runs; consider splitting into separate jobs/workflows so that the `push` path can run with least-privilege permissions (e.g., only `contents: read`). ########## .github/actions/setup-env/action.yml: ########## @@ -0,0 +1,58 @@ +# 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: 'Setup CloudStack Environment' +description: 'Sets up JDK (with Maven cache), optionally Python, and optionally APT build dependencies for CloudStack.' + +inputs: + java-version: + description: 'The JDK version to use' + required: false + default: '17' + install-python: + description: 'Whether to install Python 3.10' + required: false + default: 'false' + install-apt-deps: + description: 'Whether to install CloudStack APT build dependencies' + required: false + default: 'false' + +runs: + using: "composite" + steps: + - name: Set up JDK ${{ inputs.java-version }} + uses: actions/setup-java@v4 + with: + java-version: ${{ inputs.java-version }} + distribution: 'temurin' + architecture: x64 + cache: 'maven' + + - name: Set up Python + if: ${{ inputs.install-python == 'true' }} + uses: actions/setup-python@v5 Review Comment: The composite action uses `actions/setup-python@v5`, whereas the workflows being refactored previously referenced `actions/setup-python@v6`. If there isn’t a specific reason to pin to an older major, consider using the same major version (v6) to avoid unexpected differences in Python setup/caching behavior. ```suggestion uses: actions/setup-python@v6 ``` -- 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]
