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

SCCPPGHA-6 Support Running on Linux ARM64 #57

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

friedbyalice
Copy link
Contributor

@friedbyalice friedbyalice commented May 24, 2024

List of checkpoint to verify/do before merging a PR.

Dev checklist

  • Write/update tests
  • Functional validation
  • Public documentation update
  • Private documentation update
  • Clean commits (should start with a ticket number, clear message, no fixup, wip)
  • Maximize code coverage as much as possible

Reviewer checklist

  • Code review
  • Functional validation
  • Check commits are clean

Steps to validate this PR

Fill-in how you performed the validation, and how the reviewer can replicate it:

  • Added tests for the configure_paths script that check the newly introduced SONAR_SCANNER_URL_UNIVERSAL and SONAR_SCANNER_SHA_UNIVERSAL variables, and the variations on the other variables when running on Linux ARM64
  • Setup a repository from sonarsource-cfamily-examples with self-hosted runners (x64 and ARM64), to test that everything works on ARM64, and that java is fetched only for Linux ARM64 (the arm runner does not have java installed) run here

@friedbyalice
Copy link
Contributor Author

friedbyalice commented May 24, 2024

Currently there are no github hosted linux arm64 runners, which makes testing difficult, but a setup with self hosted runners was made on here

@friedbyalice friedbyalice force-pushed the vp/supportLinux_ARM64 branch 5 times, most recently from ea9104a to 2df64fa Compare May 24, 2024 14:10
Copy link

Choose a reason for hiding this comment

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

Minor comments.

But more importantly, it is worth mentioning that this PR also updates the sonar-scanner version somewhere, so we have improved traceability.

scripts/configure_paths.sh Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
Copy link
Member

@7PH 7PH left a comment

Choose a reason for hiding this comment

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

LGTM, with functional validation using self-hosted runners:

I see there is a ticket blocking this one on Jira about updating the SonarCloud C++ tutorials to the new syntax, let me know if you want a review there as well!

image

Copy link
Member

@7PH 7PH left a comment

Choose a reason for hiding this comment

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

I see that this pull request upgrades the sonar-scanner to v6, but the other PR does as well.

I think it is a good idea to separate the scanner bump in a different commit (as you did with the other PR), is it possible to split the work in 2 commits? (Could be in this PR or we can keep the other PR as well)

@friedbyalice friedbyalice merged commit e25edae into main Jun 24, 2024
10 checks passed
@7PH 7PH deleted the vp/supportLinux_ARM64 branch June 26, 2024 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants