in update, don't bother typechecking again if we haven't changed the unison file #4446
Conversation
| let noChanges = | ||
| and | ||
| [ UF.dataDeclarations smallUf == UF.dataDeclarations bigUf, | ||
| UF.effectDeclarations smallUf == UF.effectDeclarations bigUf, | ||
| UF.terms smallUf == UF.terms bigUf, -- no need to sort these, though it wouldn't hurt | ||
| UF.watches smallUf == UF.watches bigUf | ||
| ] |
There was a problem hiding this comment.
Seems like you should maybe just have the code that generates bigUf return a Bool of whether it had to pull in additional dependents.
Also seems like you could just compare sizes here. The bigUf can only have additional elements, since you don't remove anything, only add dependents.
There was a problem hiding this comment.
👍 I changed this to just compare sizes. Making bigBuildUnisonFile return whether it changed anything is a bit more invasive to thread that state through everywhere, so I'm not sure it's worth it.
There was a problem hiding this comment.
No objections here, just thought I'd mention it.
| and | ||
| [ Map.size (UF.dataDeclarations smallUf) == Map.size (UF.dataDeclarations bigUf), | ||
| Map.size (UF.effectDeclarations smallUf) == Map.size (UF.effectDeclarations bigUf), | ||
| length @[] (UF.terms smallUf) == length @[] (UF.terms bigUf), | ||
| Map.size (UF.watches smallUf) == Map.size (UF.watches bigUf) | ||
| ] |
There was a problem hiding this comment.
WDYT About a keysSet kind of comparison here instead of size? Then again, we don't delete anything, so this should be fine?
There was a problem hiding this comment.
Oh I see there was already discussion.
| That's done. Now I'm making sure everything typechecks... | ||
|
|
||
| Everything typechecks, so I'm saving the results... | ||
|
|
There was a problem hiding this comment.
I like that these go away.
|
Great! |
Fixes #4442