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

Testing ai pull #280

Draft
wants to merge 4 commits into
base: testingAI
Choose a base branch
from
Draft

Testing ai pull #280

wants to merge 4 commits into from

Conversation

jaci-nordic
Copy link
Contributor

No description provided.

@@ -4,7 +4,7 @@ on: pull_request

jobs:
compliance_job:
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
Copy link

Choose a reason for hiding this comment

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

The runs-on value should be a valid version of Ubuntu. ubuntu-24.04 does not exist; consider using ubuntu-22.04 or another supported version.

@@ -27,13 +27,14 @@ jobs:
run: |
pip3 install -r nrf/scripts/requirements-base.txt
pip3 install -r nrf/scripts/requirements-extra.txt
pip3 inst -r nrf/scripts/requirements-extra.txt
Copy link

Choose a reason for hiding this comment

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

Typo in the command: pip3 inst should be corrected to pip3 install.


- name: Get repo urls
working-directory: ncs
run: |
echo "SDK_ZEPHYR=$(west list zephyr -f {url} | awk -F// '{print $NF}')" >> $GITHUB_ENV
echo "SDK_MCUBOOT=$(west list mcuboot -f {url} | awk -F// '{print $NF}')" >> $GITHUB_ENV

#dummy comment
Copy link

Choose a reason for hiding this comment

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

Remove the dummy comment as it does not provide any useful information.

@jaci-nordic jaci-nordic requested review from a team as code owners January 9, 2025 13:03
@@ -16,4 +16,5 @@ tests:
- nrf52840dk/nrf52840
- nrf9160dk/nrf9160/ns
- nrf21540dk/nrf52840
- nrf3u9348934/someNewBoard
Copy link

Choose a reason for hiding this comment

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

The addition of - nrf3u9348934/someNewBoard seems arbitrary. Ensure that this entry is necessary and follows the naming conventions used for other boards.

@@ -7,7 +7,7 @@
#include "control_event.h"


static void profile_control_event(struct log_event_buf *buf,
static int profile_control_event(struct log_event_buf *buf,
Copy link

Choose a reason for hiding this comment

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

Changing the return type from void to int requires handling the return value appropriately. Ensure that all calls to profile_control_event are updated to handle the new return type.

@@ -10,7 +10,8 @@ SUIT: Recovery application
The SUIT recovery application is a minimal application that allows recovering the device firmware if the original firmware is damaged.
It is to be used as a companion firmware to the main application that is using :ref:`Software Update for Internet of Things (SUIT) <ug_nrf54h20_suit_intro>` procedure, rather than a stand-alone application.

The following limitations apply to this application:
The following limitations apply to this aptikation:
Copy link

Choose a reason for hiding this comment

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

Typo in the word 'aptikation'; it should be 'application'.

@@ -10,7 +10,8 @@
The SUIT recovery application is a minimal application that allows recovering the device firmware if the original firmware is damaged.
It is to be used as a companion firmware to the main application that is using :ref:`Software Update for Internet of Things (SUIT) <ug_nrf54h20_suit_intro>` procedure, rather than a stand-alone application.

The following limitations apply to this application:
The following limitations apply to this aptikation:
The following limitations apply to this aplication:
Copy link

Choose a reason for hiding this comment

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

Typo in the word 'aplication'; it should be 'application'.

@@ -11,7 +11,7 @@ CONFIG_SUIT_RECOVERY=y
# The MPI has to be generated from the top level application, not from
# the recovery application. This is because the digests of the main application MPI
# and the recovery application MPI is calculated together for a given domain.
CONFIG_SUIT_MPI_GENERATE=n
CONFIG_SUIT_MPI_GENERATE=nonono
Copy link

Choose a reason for hiding this comment

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

The value for CONFIG_SUIT_MPI_GENERATE should be either y or n. Using nonono is invalid and could lead to configuration errors.

@@ -157,7 +157,7 @@ def from_file(cls, filename):
configs[kconfig] = value.strip()
return configs
except Exception as err :
raise Exception("Unable to parse .config file") from err
raise Exception("Unable to parse .config file")
Copy link

Choose a reason for hiding this comment

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

The from keyword in the exception raising is useful for preserving the original traceback. Consider keeping it to aid in debugging.

@jaci-nordic jaci-nordic marked this pull request as draft January 9, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant