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

CI: More fixes and refactoring #7631

Merged
merged 4 commits into from
Feb 23, 2023
Merged

CI: More fixes and refactoring #7631

merged 4 commits into from
Feb 23, 2023

Conversation

microdev1
Copy link
Collaborator

This PR contains the following changes:

  • Split atmel-samd instead of raspberrypi: This more evenly distributes the number boards across matrix jobs.
  • Refactor and simplify fetching port deps: Now port specific stuff can be placed in .github/actions/deps/ports/....
  • Fix for empty exclude commit: Use github.event.pull_request.head.sha instead of github.event.after which can be empty.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Would you mind just splitting out every port? I split nrf out in #7497 because I needed to install nrfutil for it. See that PR for some renaming too.

@microdev1
Copy link
Collaborator Author

Would you mind just splitting out every port?

I took a look at the nrfutil changes and this pr simplifies it.

The port specific configuration steps/dependencies for nrf can go in:

# .github/actions/deps/ports/nrf/action.yml

name: Fetch nrf port deps

runs:
  using: composite
  steps:
    - name: Get nrfutil 7+
      run: |
        wget https://developer.nordicsemi.com/.pc-tools/nrfutil/x64-linux/nrfutil
        chmod +x nrfutil
        ./nrfutil install nrf5sdk-tools
        mkdir -p $HOME/.local/bin
        mv nrfutil $HOME/.local/bin
        echo "$HOME/.local/bin" >> $GITHUB_PATH
      shell: bash
    - name: Print nrfutil version
      run: nrfutil -V
      shell: bash

Then the following conditional step can be added in:

# .github/actions/deps/ports/action.yml

- name: Set up nrf port
  if: steps.board-to-port.outputs.port == 'nrf'
  uses: ./.github/actions/deps/ports/nrf

@jepler
Copy link
Member

jepler commented Feb 22, 2023

It would be unfortunate to switch to this tool as we have user/developers who build on arm64 linux platforms (e.g., rpi)...

@tannewt
Copy link
Member

tannewt commented Feb 22, 2023

It would be unfortunate to switch to this tool as we have user/developers who build on arm64 linux platforms (e.g., rpi)...

Are you talking about nrfutil? If so, #7497 is a better place to discuss this.

@microdev1 microdev1 requested a review from tannewt February 23, 2023 02:50
@tannewt
Copy link
Member

tannewt commented Feb 23, 2023

Would you mind just splitting out every port?

I took a look at the nrfutil changes and this pr simplifies it.

I agree it simplifies that part but I'd still like to split the matrix by port rather than architecture. That would be clearer than having a mix as you have now.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Approving and merging. @microdev1 will follow up with rework to each port.

@tannewt tannewt merged commit bf657e6 into adafruit:main Feb 23, 2023
@microdev1 microdev1 deleted the ci branch February 23, 2023 20:02
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.

3 participants