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

Remove arm64 build #776

Merged
merged 4 commits into from
Jul 19, 2024
Merged

Remove arm64 build #776

merged 4 commits into from
Jul 19, 2024

Conversation

danielhollas
Copy link
Contributor

@danielhollas danielhollas commented Jul 18, 2024

Closes #775

  • Removed arm64 build to reduce complexity
  • moved Dockerfile to the root directory and removed build.json and docker-build.hcl. Image can now be build by docker build . -t aiidalab/qe:newbuild
  • Only push image to ghcr.io, not to dockerhub. I think this is sufficient for this image.
  • Removed the Firefox tests, both for CI speed and simplicity. As long as we've been running these UI tests in AWB and my app, we've never caught any bug that would be specific for Firefox. Given Firefoxe's low usage, I think we can skip it here and only test with Chrome. Since these tests do not run in parallel this saves a fair amount of time (>2min). And it also makes the workflow simpler. We'll still continue testing Firefox in AWB to make sure individual widgets work there.

@danielhollas danielhollas force-pushed the remove-arm64-build branch 3 times, most recently from be1a44c to d800ab9 Compare July 18, 2024 20:07
@danielhollas danielhollas marked this pull request as ready for review July 18, 2024 20:22
@danielhollas
Copy link
Contributor Author

@unkcpz @superstar54 I need to re-enable the integration tests but otherwise this is good for initial review. Let's get this PR merged before the other.

(-400 lines of YAML and configuration!)

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.28%. Comparing base (4d7cadc) to head (6417c11).
Report is 44 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #776   +/-   ##
=======================================
  Coverage   68.28%   68.28%           
=======================================
  Files          45       45           
  Lines        4143     4143           
=======================================
  Hits         2829     2829           
  Misses       1314     1314           
Flag Coverage Δ
python-3.10 68.28% <ø> (ø)
python-3.9 68.31% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielhollas danielhollas force-pushed the remove-arm64-build branch 2 times, most recently from b30e003 to 5d768a5 Compare July 18, 2024 21:16
@danielhollas
Copy link
Contributor Author

Tests are back and passing.

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Thanks! @danielhollas Just one minor request, then good to go.

.github/workflows/docker-build-test-upload.yml Outdated Show resolved Hide resolved
@danielhollas danielhollas requested a review from unkcpz July 19, 2024 08:21
Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Approved, thanks! Let's wait tests.

@danielhollas danielhollas merged commit 71cf2ff into main Jul 19, 2024
12 checks passed
@danielhollas danielhollas deleted the remove-arm64-build branch July 19, 2024 09:04
This was referenced Jul 19, 2024
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.

Discussion: Should we build qeapp image for arm64?
2 participants