-
Notifications
You must be signed in to change notification settings - Fork 80
chore: compilation flag #1354
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
chore: compilation flag #1354
Conversation
mmagician
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for investigating this
|
I'm going to be honest, I think my understanding of this issue is very limited 😅 Is the issue here that we're passing multiple different panic strategies via compiler flags that don't agree? And this attempts to pass in this latest flag to stabilize them? I guess my question here is looking at the error linked, it shows so do we also need to modify (or remove?) any of these?
now that we're passing in
|
dagarcia7
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this! I think if the issue is fixed we should be good to merge, but just added some thoughts to give some of the other compile flags a look as well just in case they may also be part of the issue!
Thanks for taking a look! After reviewing this a bit more based on your comment, I found out why some flags are passed to I think we could just remove the nightly requirement, because I don't think we need any of the nightly stuff. Could you confirm this is the case? I am not sure if you added the first version of EDIT: Talked offline to Dennis, we decided we could try stable to see how it goes |
75cf4e4 to
9025531
Compare
|
Looks like the publishing succeeded: https://github.com/0xMiden/miden-client/actions/runs/18138878427/job/51624535483 |
I think this solves the error we've been seeing when merging to
next:See this PR for more context