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: disable prep-build-desktop in circleci #18187

Closed
wants to merge 7 commits into from

Conversation

legobeat
Copy link
Contributor

@legobeat legobeat commented Mar 16, 2023

Context

some flakiness here; from develop:

https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/39653/workflows/b78f1bc9-f3ff-4de4-9d7d-cf47f245dc57/jobs/1091593

Considering Desktop is no longer a focus so is it worth keeping the tests? This removes desktop building from the circleci pipeline.

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@legobeat legobeat marked this pull request as ready for review March 16, 2023 11:47
@legobeat legobeat requested review from a team, kumavis and brad-decker as code owners March 16, 2023 11:47
@codecov
Copy link

codecov bot commented Mar 16, 2023

Codecov Report

Merging #18187 (0364cb0) into develop (de4cf0a) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop   #18187      +/-   ##
===========================================
- Coverage    64.37%   64.37%   -0.01%     
===========================================
  Files          909      909              
  Lines        35267    35267              
  Branches      9014     9014              
===========================================
- Hits         22703    22701       -2     
- Misses       12564    12566       +2     

see 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@legobeat
Copy link
Contributor Author

CI test-deps-audit error addressed separately in #18208

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Makes sense! I don't think we need this build since the desktop integration functionality has been added to Flask.

Copy link
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

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

TBD if we're doing this.

Copy link
Contributor

@cryptotavares cryptotavares left a comment

Choose a reason for hiding this comment

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

@legobeat the flakiness does not seem to be related with the desktop build type. But rather with the build process (see circle insights). There's a success rate of 98-99% for the prep-build-desktop (which is in line with any other prep-build-<BUILD_TYPE>)

Moreover the Desktop app uses the desktop build type to build the Extension code used there (we are importing the Extension as submodule).

So If it is possible, we would like to keep this CI jobs for the desktop build type.

@legobeat legobeat closed this Mar 22, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants