Skip to content

Conversation

danielpenas42
Copy link
Contributor

@danielpenas42 danielpenas42 commented Oct 10, 2025

Implemented if statement for replace_net_uses. The tests that we checked over now do pass

@matth2k
Copy link
Owner

matth2k commented Oct 12, 2025

@danielpenas42 Thanks! The first thing you should do is fix the clippy lints (cargo clippy). Then, re-run cargo fmt.
Then, mark the PR as ready for review.

@matth2k matth2k self-requested a review October 12, 2025 18:06
@danielpenas42 danielpenas42 marked this pull request as ready for review October 15, 2025 05:59
Copy link
Owner

@matth2k matth2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Daniel! Just some small nitpicks. Also, still looks cargo clippy still has a few more lints for you to apply.

@danielpenas42
Copy link
Contributor Author

Hey I tried to fix everything and created a combinational loop to see if it gets it and from what I can tell, it is right. Lmk if anything else looks bad! I also fixed all the clippy I was running the wrong command and didnt show properly all the flaws

@danielpenas42 danielpenas42 requested a review from matth2k October 15, 2025 19:45
Copy link
Owner

@matth2k matth2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks! I had two more nitpicks but I approved so you can merge whenever you want.

@matth2k matth2k changed the title Updating /replace net uses with new test cases Updating replace_net_uses() with new test cases Oct 15, 2025
@danielpenas42
Copy link
Contributor Author

Just fixed those problems. I was wondering to merge it do i need to wait for workflow approval or can i do it straight away?

@matth2k
Copy link
Owner

matth2k commented Oct 15, 2025

Just fixed those problems. I was wondering to merge it do i need to wait for workflow approval or can i do it straight away?

Yeah it's strange I have to re-approve the CI check run every time. I will try to fix that for next time. Looks good to merge!

@matth2k matth2k merged commit 4c33660 into matth2k:main Oct 15, 2025
6 checks passed
@danielpenas42 danielpenas42 deleted the update/replace_net_uses branch October 15, 2025 22:08
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