- Notifications
You must be signed in to change notification settings - Fork44
More CI checks#93
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
4fdfc59 to99fbc4dCompareManishearth commentedFeb 29, 2024
This is still a lot of changes all in one commit. Please split it up further, it can be in one PR but it should be reviewably split into multiple commits, that have descriptive commit messages when they're making nontrivial changes. Where are all the extra tests coming from? What's with all of the changes to the python script? Some seem to be python formatting (fine), others actually change the structure/behavior of the code? |
Jules-Bertholet commentedFeb 29, 2024
The changes to the Python script are to ensure that the Rust code files it outputs pass rustfmt. I didn't make any changes to how the normalization tests are generated; I just regenerated the file, which was outdated before (as not checked by CI). |
4899ec1 toc2cb7cdCompareJules-Bertholet commentedFeb 29, 2024
I've split the PR into four commits. |
Manishearth commentedFeb 29, 2024
@Jules-Bertholet okay, can you remove the Python changes? Let's have |
And add it to `Cargo.toml`
c2cb7cd to0b9b162CompareJules-Bertholet commentedFeb 29, 2024
Done |
And also mass-reformat.`tables.rs` is ignored for now
0b9b162 tob077494CompareManishearth commentedFeb 29, 2024
CI fails |
38c395a to261ca20Compare
Splits out "unrelated" changes from#92 as requested.