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 Rust workflow for 0.1 #1064

Closed
wants to merge 2 commits into from
Closed

Remove Rust workflow for 0.1 #1064

wants to merge 2 commits into from

Conversation

stavros11
Copy link
Member

Just following #1045 (comment). From the other PR I thought that the issue is only with macos, however ubuntu also failed, so I ended up removing the whole workflow.

@stavros11 stavros11 requested a review from alecandido October 7, 2024 18:06
Copy link

codecov bot commented Oct 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.27%. Comparing base (a6049a9) to head (94b41e1).

Additional details and impacted files
@@           Coverage Diff           @@
##              0.1    #1064   +/-   ##
=======================================
  Coverage   70.27%   70.27%           
=======================================
  Files          64       64           
  Lines        6698     6698           
=======================================
  Hits         4707     4707           
  Misses       1991     1991           
Flag Coverage Δ
unittests 70.27% <ø> (ø)

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.

@alecandido
Copy link
Member

Yes, the problem seems to be the exact same (an issue in the uint32 to int32 conversion, most likely we could force the conversion in Python at call site), and now platform unrelated.

In any case, maybe we should remove not only the test, but also the crate itself, since now untested. What do you think?

@stavros11 stavros11 requested a review from hay-k October 8, 2024 09:10
@stavros11
Copy link
Member Author

stavros11 commented Oct 8, 2024

Yes, the problem seems to be the exact same (an issue in the uint32 to int32 conversion, most likely we could force the conversion in Python at call site), and now platform unrelated.

Out of curiosity, in 15dce6a I changed the return type of the Rust function to u32 and the workflow seems to be passing (https://github.com/qiboteam/qibolab/actions/runs/11234353823/job/31229984523). I also tried it locally and it worked after the fix (originally I was also getting error). Maybe this is a preferable solution compared to dropping the Rust API completely from 0.1 (also anticipating potential future releases from that branch)?

If yes, I can close this and open another PR with the fix.

@stavros11 stavros11 mentioned this pull request Oct 8, 2024
@alecandido
Copy link
Member

Since there is now #1067, we can even close this.

@stavros11
Copy link
Member Author

Since there is now #1067, we can even close this.

Indeed, closing in favor of #1067.

@stavros11 stavros11 closed this Oct 8, 2024
@stavros11 stavros11 deleted the rust-ci branch October 8, 2024 19:15
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.

2 participants