You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by "clayburn (via GitHub)" <gi...@apache.org> on 2024/01/12 15:03:44 UTC

[PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

clayburn opened a new pull request, #4740:
URL: https://github.com/apache/eventmesh/pull/4740

   <!--
   ### Contribution Checklist
   
     - Name the pull request in the form "[ISSUE #XXXX] Title of the pull request", 
       where *XXXX* should be replaced by the actual issue number.
       Skip *[ISSUE #XXXX]* if there is no associated github issue for this pull request.
   
     - Fill out the template below to describe the changes contributed by the pull request. 
       That will give reviewers the context they need to do the review.
     
     - Each pull request should address only one issue. 
       Please do not mix up code from multiple issues.
     
     - Each commit in the pull request should have a meaningful commit message.
   
     - Once all items of the checklist are addressed, remove the above text and this checklist, 
       leaving only the filled out template below.
   
   (The sections below can be removed for hotfixes of typos)
   -->
   
   <!--
   (If this PR fixes a GitHub issue, please add `Fixes #<XXX>` or `Closes #<XXX>`.)
   -->
   
   Fixes #4737.
   
   ### Motivation
   
   The motivation for this PR is to fix an issue where CodeQL is not able to analyze a PR when the Gradle build cache prevents all Java compilation. CodeQL requires the compiler to execute during the workflow in order to perform its analysis, as documented [here](https://docs.github.com/en/code-security/code-scanning/troubleshooting-code-scanning/no-source-code-seen-during-build).
   
   ### Modifications
   
   This pull request moves the CodeQL verification to its own workflow, independent of the "Continuous Integration" workflow, where Gradle build caching is disabled. This not only addresses the issue, but should also have some improvements to the workflows:
   
   * By moving CodeQL to its own workflow, it can execute in parallel. This removes CodeQL analysis from the critical path of a full build and lets the continuous integration workflow finish more quickly by leveraging build cache and skipping this analysis.
   * By moving CodeQL to its own workflow, it is no longer necessary to run the "Continuous integration" workflow for both Java and Go, since the Go branch only seemed to run in order to get CodeQL results. This would eliminate four workflows per larger build. We can also skip the Gradle build on the Go CodeQL workflow.
   
   ### Documentation
   
   - Does this pull request introduce a new feature? (yes / no)
   - If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
   - If a feature is not applicable for documentation, explain why?
   - If a feature is not documented yet in this PR, please create a followup issue for adding the documentation
   


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450662687


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}
 
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
 
-      - name: Initialize CodeQL
-        uses: github/codeql-action/init@v2
-        with:
-          # If you wish to specify custom queries, you can do so here or in a config file.
-          # By default, queries listed here will override any specified in a config file.
-          # Prefix the list here with "+" to use these queries and those in the config file.
-          # queries: ./path/to/local/query, your-org/your-repo/queries@main
-          languages: ${{ matrix.language }}
-
-      - if: matrix.language == 'cpp' || matrix.language == 'csharp'

Review Comment:
   `Build C` task was added in https://github.com/apache/eventmesh/pull/4543. It seems intentional to skip the Build C task, and there is further room for optimization (such as `submodules: true` not added yet) in this area. We have Git submodules in our C SDK, and we want to test these C codes.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450814100


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}
 
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
 
-      - name: Initialize CodeQL
-        uses: github/codeql-action/init@v2
-        with:
-          # If you wish to specify custom queries, you can do so here or in a config file.
-          # By default, queries listed here will override any specified in a config file.
-          # Prefix the list here with "+" to use these queries and those in the config file.
-          # queries: ./path/to/local/query, your-org/your-repo/queries@main
-          languages: ${{ matrix.language }}
-
-      - if: matrix.language == 'cpp' || matrix.language == 'csharp'

Review Comment:
   I have tried to run `Build C` with an additional `submodule: true` param and it encountered a compile error: https://github.com/Pil0tXia/eventmesh/actions/runs/7506142011/job/20436880958. Let's keep the original `Build C` task and I'll address it in another issue.
   
   @xwm1992 Do we still want a `Build C SDK` task?



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450809839


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}

Review Comment:
   Remaining `language: ['java']` will be better for future development.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450652081


##########
.github/workflows/codeql.yml:
##########
@@ -0,0 +1,69 @@
+#
+# 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: "CodeQL"
+
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  build:
+    name: Build
+    strategy:
+      fail-fast: false
+      matrix:
+        language: ['java', 'go']
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v2
+        with:
+          # If you wish to specify custom queries, you can do so here or in a config file.
+          # By default, queries listed here will override any specified in a config file.
+          # Prefix the list here with "+" to use these queries and those in the config file.
+          # queries: ./path/to/local/query, your-org/your-repo/queries@main
+          languages: ${{ matrix.language }}
+
+      - name: Setup Gradle
+        uses: gradle/gradle-build-action@v2
+        if: matrix.language == 'java'
+
+      - name: Set up JDK 11
+        uses: actions/setup-java@v3
+        with:
+          distribution: 'zulu'
+          java-version: 11
+        if: matrix.language == 'java'
+
+      # https://docs.gradle.org/current/userguide/performance.html
+      - name: Build
+        run: ./gradlew clean assemble compileTestJava --no-build-cache --parallel --daemon

Review Comment:
   Based on our previous experience, we would like to execute the `build` task. The `build` task should already include the `assemble` and `compileTestJava` tasks. In your experience, do you think it's necessary to replace the `build` task with the `assemble` and `compileTestJava` tasks?



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#issuecomment-1889570238

   ## [Codecov](https://app.codecov.io/gh/apache/eventmesh/pull/4740?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   All modified and coverable lines are covered by tests :white_check_mark:
   > Comparison is base [(`aae0d54`)](https://app.codecov.io/gh/apache/eventmesh/commit/aae0d540ae02e9150bbec455caa460fc9189eca8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 17.60% compared to head [(`3a79ae1`)](https://app.codecov.io/gh/apache/eventmesh/pull/4740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 17.59%.
   
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #4740      +/-   ##
   ============================================
   - Coverage     17.60%   17.59%   -0.02%     
   + Complexity     1775     1774       -1     
   ============================================
     Files           797      797              
     Lines         29786    29786              
     Branches       2573     2574       +1     
   ============================================
   - Hits           5243     5240       -3     
   - Misses        24063    24065       +2     
   - Partials        480      481       +1     
   ```
   
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/eventmesh/pull/4740?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#issuecomment-1889548996

   This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on [this overview](/apache/eventmesh/security/code-scanning?query=pr%3A4740+is%3Aopen). Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out [the documentation](https://docs.github.com/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/about-code-scanning). 


-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "clayburn (via GitHub)" <gi...@apache.org>.
clayburn commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450575566


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}
 
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
 
-      - name: Initialize CodeQL
-        uses: github/codeql-action/init@v2
-        with:
-          # If you wish to specify custom queries, you can do so here or in a config file.
-          # By default, queries listed here will override any specified in a config file.
-          # Prefix the list here with "+" to use these queries and those in the config file.
-          # queries: ./path/to/local/query, your-org/your-repo/queries@main
-          languages: ${{ matrix.language }}
-
-      - if: matrix.language == 'cpp' || matrix.language == 'csharp'

Review Comment:
   This step seemed like it would never execute, as neither of these languages was in the matrix, so I did not migrate it to the CodeQL workflow.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450756614


##########
.github/workflows/codeql.yml:
##########
@@ -0,0 +1,69 @@
+#
+# 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: "CodeQL"
+
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  build:
+    name: Build
+    strategy:

Review Comment:
   May you please change CodeQL's job name to `Analyze` or something else? This way, the tasks related to CodeQL will be easier to distinguish in all workflows.
   
   If you're willing to, could you please change the workflow name and filename of `build.yaml` to "Docker" and "docker.yml" respectively? This modification is unrelated to this PR, but it can be done along with it.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450841267


##########
.github/workflows/codeql.yml:
##########
@@ -0,0 +1,69 @@
+#
+# 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: "CodeQL"
+
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  build:
+    name: Build
+    strategy:

Review Comment:
   Excuse me for any confusion. I mean "changing CodeQL's **job** name to `Analyze`" (located at L30 in this conversation's reviewed code block) instead of its **workflow** name (L20).
   
   To be exactly, from `CodeQL/Build` to `CodeQL/Analyze`. The current version is `Analyze/Build`.
   
   ![image](https://github.com/apache/eventmesh/assets/41445332/1e84250c-9ea3-4074-be61-4fdab4bca196)
   



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450748752


##########
.github/workflows/codeql.yml:
##########
@@ -0,0 +1,69 @@
+#
+# 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: "CodeQL"
+
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  build:
+    name: Build
+    strategy:
+      fail-fast: false
+      matrix:
+        language: ['java', 'go']
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v2
+        with:
+          # If you wish to specify custom queries, you can do so here or in a config file.
+          # By default, queries listed here will override any specified in a config file.
+          # Prefix the list here with "+" to use these queries and those in the config file.
+          # queries: ./path/to/local/query, your-org/your-repo/queries@main
+          languages: ${{ matrix.language }}
+
+      - name: Setup Gradle
+        uses: gradle/gradle-build-action@v2
+        if: matrix.language == 'java'
+
+      - name: Set up JDK 11
+        uses: actions/setup-java@v3
+        with:
+          distribution: 'zulu'
+          java-version: 11
+        if: matrix.language == 'java'
+
+      # https://docs.gradle.org/current/userguide/performance.html
+      - name: Build
+        run: ./gradlew clean assemble compileTestJava --no-build-cache --parallel --daemon

Review Comment:
   OK, let's keep `assemble compileTestJava` then. This way, only the source code will be compiled, but CodeQL doesn't need them to run.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "clayburn (via GitHub)" <gi...@apache.org>.
clayburn commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450845102


##########
.github/workflows/codeql.yml:
##########
@@ -0,0 +1,69 @@
+#
+# 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: "CodeQL"
+
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  build:
+    name: Build
+    strategy:

Review Comment:
   Ah sorry, you said it clearly and I misunderstood. Will update this



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450814100


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}
 
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
 
-      - name: Initialize CodeQL
-        uses: github/codeql-action/init@v2
-        with:
-          # If you wish to specify custom queries, you can do so here or in a config file.
-          # By default, queries listed here will override any specified in a config file.
-          # Prefix the list here with "+" to use these queries and those in the config file.
-          # queries: ./path/to/local/query, your-org/your-repo/queries@main
-          languages: ${{ matrix.language }}
-
-      - if: matrix.language == 'cpp' || matrix.language == 'csharp'

Review Comment:
   I have tried to run `Build C` with an additional `submodule: true` param and it encountered a compile error: https://github.com/Pil0tXia/eventmesh/actions/runs/7506142011/job/20436880958. Let's keep the original `Build C` task and I'll address it in https://github.com/apache/eventmesh/issues/4742 and https://github.com/apache/eventmesh/pull/4743.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "clayburn (via GitHub)" <gi...@apache.org>.
clayburn commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450723496


##########
.github/workflows/codeql.yml:
##########
@@ -0,0 +1,69 @@
+#
+# 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: "CodeQL"
+
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  build:
+    name: Build
+    strategy:
+      fail-fast: false
+      matrix:
+        language: ['java', 'go']
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v2
+        with:
+          # If you wish to specify custom queries, you can do so here or in a config file.
+          # By default, queries listed here will override any specified in a config file.
+          # Prefix the list here with "+" to use these queries and those in the config file.
+          # queries: ./path/to/local/query, your-org/your-repo/queries@main
+          languages: ${{ matrix.language }}
+
+      - name: Setup Gradle
+        uses: gradle/gradle-build-action@v2
+        if: matrix.language == 'java'
+
+      - name: Set up JDK 11
+        uses: actions/setup-java@v3
+        with:
+          distribution: 'zulu'
+          java-version: 11
+        if: matrix.language == 'java'
+
+      # https://docs.gradle.org/current/userguide/performance.html
+      - name: Build
+        run: ./gradlew clean assemble compileTestJava --no-build-cache --parallel --daemon

Review Comment:
   `build` will run everything in the build, most notably the tests. The tests will be verified in the Continuous Integration workflow, so running them here would be redundant (unless I misunderstand and CodeQL needs to see them execute). Running `assemble compileTestJava` will just compile main and test source set that CodeQL cares about.
   
   If you find it better, we could change this to something like `build -x test` for a similar result.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "clayburn (via GitHub)" <gi...@apache.org>.
clayburn commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450720494


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}
 
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
 
-      - name: Initialize CodeQL
-        uses: github/codeql-action/init@v2
-        with:
-          # If you wish to specify custom queries, you can do so here or in a config file.
-          # By default, queries listed here will override any specified in a config file.
-          # Prefix the list here with "+" to use these queries and those in the config file.
-          # queries: ./path/to/local/query, your-org/your-repo/queries@main
-          languages: ${{ matrix.language }}
-
-      - if: matrix.language == 'cpp' || matrix.language == 'csharp'

Review Comment:
   > You mean L52~L57 task
   
   Yes, the conditional check for cpp/csharp
   
   > Build C task was added in https://github.com/apache/eventmesh/pull/4543. It seems intentional to skip the Build C task, and there is further room for optimization (such as submodules: true not added yet) in this area. We have Git submodules in our C SDK, and we want to test these C codes.
   
   I'm not quite sure what you mean here, since the way this is currently authored, matrix.language will never be either of these two options. So this is essentially a dead code path as it exists today. But I can add it back if it is an area of future change.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450814100


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}
 
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
 
-      - name: Initialize CodeQL
-        uses: github/codeql-action/init@v2
-        with:
-          # If you wish to specify custom queries, you can do so here or in a config file.
-          # By default, queries listed here will override any specified in a config file.
-          # Prefix the list here with "+" to use these queries and those in the config file.
-          # queries: ./path/to/local/query, your-org/your-repo/queries@main
-          languages: ${{ matrix.language }}
-
-      - if: matrix.language == 'cpp' || matrix.language == 'csharp'

Review Comment:
   I have tried to run `Build C` with an additional `submodule: true` param and it encountered a compile error: https://github.com/Pil0tXia/eventmesh/actions/runs/7506142011/job/20436880958. Let's keep the original `Build C` task and I'll address it in another issue.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "xwm1992 (via GitHub)" <gi...@apache.org>.
xwm1992 merged PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "clayburn (via GitHub)" <gi...@apache.org>.
clayburn commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450577031


##########
.github/workflows/codeql.yml:
##########
@@ -0,0 +1,69 @@
+#
+# 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: "CodeQL"
+
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  build:
+    name: Build
+    strategy:
+      fail-fast: false
+      matrix:
+        language: ['java', 'go']
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v2
+        with:
+          # If you wish to specify custom queries, you can do so here or in a config file.
+          # By default, queries listed here will override any specified in a config file.
+          # Prefix the list here with "+" to use these queries and those in the config file.
+          # queries: ./path/to/local/query, your-org/your-repo/queries@main
+          languages: ${{ matrix.language }}
+
+      - name: Setup Gradle
+        uses: gradle/gradle-build-action@v2
+        if: matrix.language == 'java'
+
+      - name: Set up JDK 11
+        uses: actions/setup-java@v3
+        with:
+          distribution: 'zulu'
+          java-version: 11
+        if: matrix.language == 'java'
+
+      # https://docs.gradle.org/current/userguide/performance.html
+      - name: Build
+        run: ./gradlew clean assemble compileTestJava --no-build-cache --parallel --daemon

Review Comment:
   I am not familiar with your usage of CodeQL, so I made the assumption that you used it to scan test code as well. If you do not, the `compileTestJava` task can be removed here.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450633604


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}
 
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
 
-      - name: Initialize CodeQL
-        uses: github/codeql-action/init@v2
-        with:
-          # If you wish to specify custom queries, you can do so here or in a config file.
-          # By default, queries listed here will override any specified in a config file.
-          # Prefix the list here with "+" to use these queries and those in the config file.
-          # queries: ./path/to/local/query, your-org/your-repo/queries@main
-          languages: ${{ matrix.language }}
-
-      - if: matrix.language == 'cpp' || matrix.language == 'csharp'

Review Comment:
   You mean L52~L57 task `Build C` or L50 `languages: ${{ matrix.language }}`?



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450633604


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}
 
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
 
-      - name: Initialize CodeQL
-        uses: github/codeql-action/init@v2
-        with:
-          # If you wish to specify custom queries, you can do so here or in a config file.
-          # By default, queries listed here will override any specified in a config file.
-          # Prefix the list here with "+" to use these queries and those in the config file.
-          # queries: ./path/to/local/query, your-org/your-repo/queries@main
-          languages: ${{ matrix.language }}
-
-      - if: matrix.language == 'cpp' || matrix.language == 'csharp'

Review Comment:
   You mean L52 `if: matrix.language == 'cpp' || matrix.language == 'csharp'` or L50 `languages: ${{ matrix.language }}`?



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "clayburn (via GitHub)" <gi...@apache.org>.
clayburn commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450778114


##########
.github/workflows/codeql.yml:
##########
@@ -0,0 +1,69 @@
+#
+# 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: "CodeQL"
+
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  build:
+    name: Build
+    strategy:

Review Comment:
   Pushed a change that does both



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


Re: [PR] [ISSUE #4737] Separate codeql workflow (eventmesh)

Posted by "clayburn (via GitHub)" <gi...@apache.org>.
clayburn commented on code in PR #4740:
URL: https://github.com/apache/eventmesh/pull/4740#discussion_r1450812106


##########
.github/workflows/ci.yml:
##########
@@ -33,29 +33,12 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macOS-latest ]
         java: [ 8, 11 ]
-        language: ['java', 'go']
     runs-on: ${{ matrix.os }}

Review Comment:
   Would you like me to restore the `if: matrix.language == 'cpp' || matrix.language == 'csharp'` step as well with that?



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org