-
Notifications
You must be signed in to change notification settings - Fork 129
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
fix: correct the pub types/fields(post frontend-backend split) #276
Conversation
@duguorong009 CI is not passing. Could you address the errors first? |
@CPerezz Fixed the CI issues. |
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.
Everything looks good except for the two comments I added.
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 addressing the feedback! I totally agree that we can do more corrections once #266 is completed :)
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.
LGTM modulo filling an issue for the TODO.
@@ -3,6 +3,7 @@ use halo2_middleware::ff::Field; | |||
use std::fmt::{self, Debug}; | |||
|
|||
/// Expressions involved in a lookup argument, with a name as metadata. | |||
/// TODO: possible to move to "halo2_backend", if moved, pub(crate) fields. |
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.
Do we have an issue for that?
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.
This issue is related to #266 .
FYI, I added the TODO
comment for easy lookup.
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.
For the best, I added this issue to list of #266.
#266 (comment)
Description
Related issues
Changes
halo2_common
,halo2_frontend
,halo2_backend