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.

Sign up for GitHub

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

AyabAsync v1.1.3 #209

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

AyabAsync v1.1.3 #209

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3533d16
Update archive_build.yml
jpcornil-git Nov 2, 2024
6c8b128
Upload artifact from ayabAsync v1.1.1
jpcornil-git Nov 8, 2024
2e38900
Update archive_build.yml
jpcornil-git Nov 8, 2024
d7f83ed
Update archive_build.yml
jpcornil-git Nov 8, 2024
f4c7ab0
Update to v1.1.2
jpcornil-git Nov 13, 2024
ac56aa6
v1.1.2
jpcornil-git Nov 13, 2024
0842508
v1.1.3
jpcornil-git Nov 17, 2024
ece5793
Update to actions/upload-artifact@v4 to remove warnings
jpcornil-git Nov 17, 2024
bd178c6
Merge branch 'main' into Share
jpcornil-git Nov 18, 2024
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
13 changes: 3 additions & 10 deletions .github/workflows/archive_build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,10 @@ jobs:
archive:
runs-on: ubuntu-latest
steps:
- name: Checkout repo and submodules
uses: actions/checkout@v3
with:
submodules: recursive
fetch-depth: 0
- name: Build Firmware
uses: ./.github/actions/build
with:
BUILD_WRAPPER_OUT_DIR: build_wrapper_out_dir
- name: Fetch binary
run: wget https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/v1.1.0/firmware_uno_1_1_0.hex
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for wget command.

The wget command lacks error handling which could lead to silent failures.

Implement proper error handling:

 - name: Fetch binary
-  run: wget https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/v1.1.0/firmware_uno_1_1_0.hex
+  run: |
+    set -e
+    if ! wget --spider https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/v1.1.0/firmware_uno_1_1_0.hex; then
+      echo "Error: Firmware file not found"
+      exit 1
+    fi
+    wget https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/v1.1.0/firmware_uno_1_1_0.hex
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Fetch binary
run: wget https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/v1.1.0/firmware_uno_1_1_0.hex
- name: Fetch binary
run: |
set -e
if ! wget --spider https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/v1.1.0/firmware_uno_1_1_0.hex; then
echo "Error: Firmware file not found"
exit 1
fi
wget https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/v1.1.0/firmware_uno_1_1_0.hex

🛠️ Refactor suggestion

Use environment variables or GitHub Actions inputs for version number.

The version number (v1.1.0) is hardcoded in the URL, making it difficult to maintain and update.

Consider this improvement:

+env:
+  AYAB_VERSION: v1.1.0
+
 steps:
   - name: Fetch binary
-    run: wget https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/v1.1.0/firmware_uno_1_1_0.hex
+    run: wget https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/${{ env.AYAB_VERSION }}/firmware_uno_${{ env.AYAB_VERSION }}.hex

Committable suggestion skipped: line range outside the PR's diff.

⚠️ Potential issue

Add checksum verification for the downloaded binary.

Downloading binaries without verification poses a security risk. Consider adding SHA256/SHA512 checksum validation.

Here's a suggested implementation:

 - name: Fetch binary
   run: |
     wget https://github.com/jpcornil-git/ayabAsync-firmware/releases/download/v1.1.0/firmware_uno_1_1_0.hex
+    echo "expected_sha256_hash firmware_uno_1_1_0.hex" | sha256sum --check

Please replace expected_sha256_hash with the actual SHA256 hash of the firmware file.

Committable suggestion skipped: line range outside the PR's diff.

- name: Archive build
uses: actions/upload-artifact@v3
with:
name: ${{format('{0}-{1}',github.sha,github.run_number)}}
path: build/*.hex
path: ./*.hex
Comment on lines 29 to +30
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Verify hex file existence before archiving.

The workflow might succeed even if the hex file wasn't downloaded successfully.

Add a verification step:

+      - name: Verify hex file
+        run: |
+          if [ ! -f ./*.hex ]; then
+            echo "Error: No hex file found"
+            exit 1
+          fi
       - name: Archive build
         uses: actions/upload-artifact@v3
         with:

Committable suggestion skipped: line range outside the PR's diff.

Loading