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

Jump to bottom

Feature/irnas zephyr template migration #14

Merged
merged 2 commits into from
Dec 20, 2023
Merged

Feature/irnas zephyr template migration #14

merged 2 commits into from
Dec 20, 2023

Conversation

NejcKle
Copy link
Contributor

@NejcKle NejcKle commented Dec 13, 2023 •
edited
Loading

Description

This PR implements changes required to migrate the workflow files and scripts from the irnas-runners-software repository to this one and irnas-workflows-software. Some files got moved around and/or to different repositories, see Related below.

Changes:

  • Workflow files have been modified to run on self-hosted runners.
  • The makefile has been modified to include the remote twister runner task.
  • Relevant scripts related to remote Raspberry Pi twister runner have been included.
  • Docs have been updated.

Closes #

Related https://github.com/IRNAS/irnas-runners-software/pull/6 IRNAS/irnas-workflows-software#9

Areas of interest for the reviewer

Check the workflow files and docs.

Checklist

  • My code follows the style guidelines as defined by IRNAS.
  • I have performed a self-review of my code.
  • My changes generate no new warnings.
  • I added/updated source code documentation for all newly added or changed
    functions.
  • I updated all customer-facing technical documentation.

After-review steps

  • Reviewer can merge and delete the branch.

@github-actions github-actions bot added the pull request Pull request, added automatically by CI. label Dec 13, 2023
@NejcKle NejcKle mentioned this pull request Dec 13, 2023
Merged
5 tasks
MarkoSagadin
MarkoSagadin requested changes Dec 18, 2023
View reviewed changes
Copy link
Collaborator

@MarkoSagadin MarkoSagadin left a comment

Choose a reason for hiding this comment

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

Looks good I have marked some small things. Some of the requests are probably also relevant to the other two repos, so I will not mention them in the other PRs.

One general thing: Be consistent with the period dots in the comments. Some of them are missing it. Please go through the comments and add them.

.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/codechecker.yaml Outdated
Comment on lines 84 to 85
# The retrieve cache job will run only, if the host is not a ubuntu-20.04 runner.
# GitHub hosted runners begin with 'Github Action'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this comment. Please do the same in all similarly relevant places.

.github/workflows/twister.yaml Outdated Show resolved Hide resolved
.github/workflows/twister.yaml Outdated Show resolved Hide resolved
.github/workflows/twister.yaml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
scripts/rpi-jlink-server/README.md Outdated Show resolved Hide resolved
MarkoSagadin
MarkoSagadin requested changes Dec 20, 2023
View reviewed changes
Copy link
Collaborator

@MarkoSagadin MarkoSagadin left a comment

Choose a reason for hiding this comment

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

Marked some minor things in the comments

.github/workflows/twister.yaml Outdated
# This version is a must, twister otherwise fails (some Python library
# depends on a specific version of libffi that is not present in the
# toolchain provided by nordic's toolchain manager).
runs-on: ubuntu-22.04
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is now not true. The original comment was referring to the ubuntu-20.04, and that was for the twister-build job.

Please place this comment in the docker image inside irnas-runners-software repo, as our self-hosted will be running ubuntu-20.04, so that is the most appropriate place for it (since ubuntu-22.04 and nrftutil-toolchain-manager seem to be incompatible for now).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, above comment starting with WARNING is also not true since job twister-test-results uses ubuntu-22.04 instead of self-hosted. Am I missing something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see now. Also, I have reworded the warning to be more concise.

@NejcKle NejcKle force-pushed the feature/irnas-zephyr-template-migration branch from f575314 to 998a599 Compare December 20, 2023 11:09
@MarkoSagadin MarkoSagadin self-requested a review December 20, 2023 11:23
@NejcKle NejcKle merged commit f6b15d6 into main Dec 20, 2023
0 of 2 checks passed
@NejcKle NejcKle deleted the feature/irnas-zephyr-template-migration branch December 20, 2023 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarkoSagadin MarkoSagadin MarkoSagadin approved these changes

Assignees
No one assigned
Labels
pull request Pull request, added automatically by CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NejcKle @MarkoSagadin