search

NethermindEth / warp

Warp - Bringing Solidity to Starknet at warp speed. Warp is a Solidity to Cairo Compiler, this allows teams to write/migrate Solidity to Cairo for easy onboarding into the StarkNet ecosystem.
https://nethermind.io/warp/
Apache License 2.0
754 stars 70 forks source link

Check for passing behavior tests #1055

Closed AlejandroLabourdette closed 1 year ago

piwonskp commented 1 year ago

Can we remove tests that don't pass at the moment, or move them to a different file? In the first approach, we can resurrect fixed tests from older revisions. As it is now, it's hard to see what has been uncommented and is working and the diff blowup is impressive

I think the huge diff is an issue of indentation change, or sth like that in the PR. The diff should only include uncommented tests so it shouldn't be that huge

temyurchenko commented 1 year ago

Can we remove tests that don't pass at the moment, or move them to a different file? In the first approach, we can resurrect fixed tests from older revisions. As it is now, it's hard to see what has been uncommented and is working and the diff blowup is impressive

I think the huge diff is an issue of indentation change, or sth like that in the PR. The diff should only include uncommented tests so it shouldn't be that huge

I see. Then the indentation change should preferably be fixed, otherwise it is genuinely hard to navigate thousands of lines of diffs

piwonskp commented 1 year ago

Yea I think what has changed is the indentation of // comments, previously they weren't indented, now they are

temyurchenko commented 1 year ago

I tried to reduce the effect of comments in #1073 as much as possible. I only touched places where the only change is placement of comments. There were some reorderings and renamings that could probably also be reverted, but I didn't touch them, I don't understand it that well.