Add phased subcircuit autorouting#2127
Add phased subcircuit autorouting#2127ShiboSoftwareDev wants to merge 2 commits intotscircuit:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
seveibar
left a comment
seveibar
left a comment
There was a problem hiding this comment.
this is too confusing as written, but it works so it's a good first attempt
the diff is really hard to read because you used closures. You should request changes for any PR that has closures- we don't accept them because they cause a codebase to rapidly become unmaintanable because of scope leaking
| return traces.length > 0 | ||
| } | ||
|
|
||
| _getRoutingPhasePlans(): RoutingPhasePlan[] { |
There was a problem hiding this comment.
should this be in the subcircuit class?
| }> = [] | ||
| let obstacleCircuitJson: AnyCircuitElement[] = [] | ||
|
|
||
| const solvePhase = async (phasePlan: RoutingPhasePlan) => { |
There was a problem hiding this comment.
so generally, named closures are a huge antipattern, for some reason codex loves making them, but we'll rarely accept a PR with them, and staff are instructed to make sure we don't merge code with named closures especially into core
| <board width="24mm" height="20mm"> | ||
| <subcircuit | ||
| name="POWER_STAGE" | ||
| autorouter={{ local: true, groupMode: "subcircuit" }} |
There was a problem hiding this comment.
| autorouter={{ local: true, groupMode: "subcircuit" }} |
|
|
||
| circuit.add( | ||
| <board width="24mm" height="20mm"> | ||
| <subcircuit |
There was a problem hiding this comment.
remove the subcircuit- why is there a subcircuit?
|
@ShiboSoftwareDev you might want to do 2 PRs, in the first PR you can just "set up" the autorouting system to avoid some kind of large refactor. The diff is impossible to read as is and this is sensitive logic. If the code was broken up properly there would not be this issue, so you might want to clean up the functions and do basic methods in the first PR, then do the actual implementation in a second PR. Being careful and keeping this code clean will save us hundreds of hours in the long term |
|
This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs. |
2 participants
No description provided.