Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/identity #811

Merged
merged 27 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions .github/workflows/auto-pr.yml

This file was deleted.

50 changes: 9 additions & 41 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,70 +16,38 @@ on:
pull_request:
branches:
- "**"
push:
branches:
- "v[0-9]**"
workflow_dispatch:
merge_group:
env:
GO_VERSION: "1.20.5"
workflow_dispatch:
workflow_call:

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true

jobs:
build-default:
name: Build sonrd-${{ matrix.targetos }}-${{ matrix.arch }}
runs-on: ubuntu-latest

name: Build sonrd-${{ matrix.targetos }}-${{ matrix.arch }} for ${{ github.ref }}
runs-on: self-hosted
strategy:
matrix:
arch: [amd64, arm64]
targetos: [darwin, linux]
include:
- targetos: darwin
arch: arm64

steps:
- name: Check out repository code
uses: actions/checkout@v3
- name: Get git diff
uses: technote-space/[email protected]
with:
PATTERNS: |
**/**.wasm
**/**.go
go.mod
go.sum
Makefile
.github/workflows/build.yml

- uses: actions/setup-go@v4
if: env.GIT_DIFF
with:
go-version: ${{env.GO_VERSION}}
env:
GOOS: ${{ matrix.targetos }}
GOARCH: ${{ matrix.arch }}

- name: Download Dependencies
if: env.GIT_DIFF
run: go mod download
- name: Build sonrd
if: env.GIT_DIFF
run: |
GOWRK=off go build cmd/sonrd/main.go
- name: Upload sonrd artifact
if: env.GIT_DIFF
uses: actions/upload-artifact@v3
- run: make build
- uses: actions/upload-artifact@v3
with:
name: sonrd-${{ matrix.targetos }}-${{ matrix.arch }}
path: cmd/sonrd/sonrd

build-make:
name: Build using makefile
runs-on: ubuntu-latest
steps:
- name: Check out repository code
uses: actions/checkout@v3
- name: Run make build
run: make build
path: bin/sonrd
2 changes: 1 addition & 1 deletion .github/workflows/changelog.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ permissions:

jobs:
generate-changelog:
runs-on: ubuntu-latest
runs-on: ubuntu-ultra-instinct
steps:
- uses: actions/checkout@v2
with:
Comment on lines 10 to 16

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from "ubuntu-latest" to "ubuntu-ultra-instinct" in the "runs-on" field of the job looks like a naming convention change. However, it is always preferable to use the available official environment labels provided by GitHub. In this case, "ubuntu-latest" is the standard label, so it should be retained.

jobs:
  generate-changelog:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
        with:

By sticking to the official environment labels, it ensures consistency and makes it easier to understand and maintain the code in the long run.

Expand Down
72 changes: 0 additions & 72 deletions .github/workflows/cleanup.yml

This file was deleted.

46 changes: 20 additions & 26 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
name: "🧪 CodeQL Analysis"

on:
pull_request:
branches: [ master ]
schedule:
- cron: "43 3 * * 2"

jobs:
codeql-analysis:
name: Run CodeQL Analysis
Comment on lines 1 to 10

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback:

  1. It is good to see that you have added a name field to your workflow.
  2. I would recommend removing the schedule field as it is not necessary for a pull_request event.
  3. It is best practice to include some context in the cron schedule, such as an explanation of when the analysis will run.

Here is the updated code:

name: "🧪 CodeQL Analysis"
 
on:
  pull_request:
    branches: [ master ]

jobs:
  codeql-analysis:
    name: Run CodeQL Analysis

Expand All @@ -12,44 +13,37 @@ jobs:
actions: read
contents: read
security-events: write

strategy:
fail-fast: false
matrix:
language: ["go", "typescript"]
queries: ['crypto-com/[email protected]']
# CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby' ]
# Learn more about CodeQL language support at https://git.io/codeql-language-support

steps:
- name: Checkout repository
uses: actions/checkout@v3

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@v1
with:
languages: ${{ matrix.language }}
# 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

# Autobuild attempts to build any compiled languages (C/C++, C#, or Java).
# If this step fails, then you should remove it and run the build manually (see below)
- name: Autobuild
uses: github/codeql-action/autobuild@v1

# ℹ️ Command-line programs to run using the OS shell.
# 📚 https://git.io/JvXDl

# ✏️ If the Autobuild fails above, remove it and uncomment the following three lines
# and modify them (or add more) to build your code if your project
# uses a compiled language

#- run: |
# make bootstrap
# make release

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v1

coverage:
name: Scan/Upload Coverage
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v3
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

- name: SonarCloud Scan
uses: sonarsource/sonarcloud-github-action@master
env:
GITHUB_TOKEN: ${{ secrets.GH_ACTIONS_PAT }}
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
Comment on lines 13 to +49

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the code changes look good, but here are a few suggestions for improvement:

  1. Move the coverage block outside the steps block in order to maintain a clear separation of concerns and improve readability.

    coverage:
      name: Scan/Upload Coverage
      runs-on: ubuntu-latest
      steps:
        # ...
  2. Use the env block under the jobs key to define environment variables instead of using env within each step.

    jobs:
      build:
        env:
          CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
          GITHUB_TOKEN: ${{ secrets.GH_ACTIONS_PAT }}
          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
        steps:
          # ...
  3. Consider using an explicit version of the sonarsource/sonarcloud-github-action instead of @master to ensure a specific version is used.

    - name: SonarCloud Scan
      uses: sonarsource/sonarcloud-github-action@<version>
      env:
        GITHUB_TOKEN: ${{ secrets.GH_ACTIONS_PAT }}
        SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
  4. Add comments to explain the purpose of each step to increase code readability and maintainability.

    - name: Checkout repository
      uses: actions/checkout@v3
       
    # ...
    
    - name: Initialize CodeQL
      uses: github/codeql-action/init@v1
      with:
        languages: ${{ matrix.language }}
    
    # ...

These changes help improve the readability, organization, and maintainability of the code.

47 changes: 0 additions & 47 deletions .github/workflows/hadolint.yml

This file was deleted.

2 changes: 1 addition & 1 deletion .github/workflows/labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ on: [pull_request]

jobs:
label:
runs-on: ubuntu-latest
runs-on: self-hosted
permissions:
contents: read
pull-requests: write
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/proto.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ permissions:

jobs:
lint:
runs-on: ubuntu-latest
runs-on: self-hosted
timeout-minutes: 5
steps:
- uses: actions/checkout@v4
Expand All @@ -21,7 +21,7 @@ jobs:
input: "proto"

break-check:
runs-on: ubuntu-latest
runs-on: self-hosted
steps:
- uses: actions/checkout@v4
- uses: bufbuild/[email protected]
Expand Down
17 changes: 0 additions & 17 deletions .github/workflows/publish.yml

This file was deleted.

6 changes: 4 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ permissions:
jobs:
release:
name: Run goreleaser
runs-on: ubuntu-latest
runs-on: self-hosted
steps:
- uses: actions/checkout@v3
with:
Comment on lines 17 to 23

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes look good, but it would be better if we specify a specific version of the self-hosted runner instead of using "self-hosted". This will ensure consistency and prevent any unexpected issues from using different versions of the self-hosted runner.

jobs:
  release:
    name: Run goreleaser
    runs-on: self-hosted[VERSION_TAG]
    steps:
      - uses: actions/checkout@v3
        with:

Replace [VERSION_TAG] with the specific version of the self-hosted runner you want to use.

Expand All @@ -26,9 +26,11 @@ jobs:
- run: make release
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

publish-buf:
name: Publish buf.build proto
runs-on: ubuntu-latest
runs-on: self-hosted
needs: release
steps:
- uses: actions/checkout@v3
with:
Comment on lines 26 to 36

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback:

  1. The run command for make release is missing the necessary arguments or commands. Make sure to include the appropriate command for the release process.

  2. It's good to specify the runs-on as self-hosted if you are using a self-hosted runner. However, make sure that the self-hosted runner has the necessary dependencies and configurations to handle the job.

  3. It's important to specify the dependency needs: release if the publish-buf job depends on the release job.

Code example:

       - run: make release <arguments>
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

   publish-buf:
     name: Publish buf.build proto
     runs-on: self-hosted
     needs: release
     steps:
       - uses: actions/checkout@v3
         with:
         ...

Note: Replace <arguments> with the actual arguments or commands needed for the release process.

Expand Down
Loading
Loading