Revamp `update` process
https://gist.github.com/pchiusano/548032b98d5d6b29cb4314421a231a42 has a writeup of the problem we are trying to solve and proposed new workflow.
Old commentary, out of date
A couple things I am aware of:
- It’s very easy to accidentally add updates to the wrong patch, and in general the update process seems fully of footguns when it should be a delightful improvement over the status quo.
- When you goof up a patch, it’s annoying to fix if you notice much later.
- I am pretty sure there is a bug with
todowhere it doesn’t report things that depend transitively on the mapped definitions in patch, only the things that depend directly on the mapped definitions. This can lead to confusing results where an update propagates to immediate dependents, which are able to typecheck, but when their dependents can’t typecheck, that isn’t reported as a todo, even though they still depend on an old hash. - Issue with lack of propagation of type edits, but that is a more narrow technical problem being tracked by #2188
- Patches should probably have the invariant that their RHS is named in the current namespace (see this comment) and also perhaps we want a
patch.cleanup lastrelease trunk.patchwhich also culls out any entries whose LHS isn't mentioned inlastrelease. In this way you might restrict a patch to just talk about upgrades to definitions from the last release of a library; anything else is just local churn.
Phase 1 of this is just a design jam. cc @rlmark @runarorama @stew @hojberg
—
Misc ideas from @pchiusano
When you update foo, it adds the entries to the patch but also adds old definition to the namespace foo, adding .deprecated onto the end of their names. Propagation also adds definitions to foo.
todo foo then reports if stuff depends on anything in foo in addition to checking for patch conflicts for the foo patch and any name conflicts in the current namespace. When you’re done, you just delete.namespace foo.
This idea of putting the in-progress, deprecated definitions into a namespace that you delete when you’re done (possibly todo command can suggest this if there’s nothing todo) feels like a nicer experience during the refactoring process: you can see the code you’re refactoring depends on myRefactoring.blah.deprecated rather than like blah#303489a.
I’m not even sure that we need “old names” support at all with this design. In general, I think showing people a name plus a hash is not a good experience and we should try not to do that were possible - people are going to want more context.
I think we need to revamp the "current namespace" UX and that'll help with number 1. I'm looking for an existing ticket on this, but can't find one.
Propagation also adds definitions to
foo.
What does this mean? Ah, the auto-propagate kind of update. If bar depends on foo and you update foo, then it creates a foo.bar.deprecated?
In general, I think showing people a name plus a hash is not a good experience and we should try not to do that were possible - people are going to want more context.
Agreed.
Questions: What happens here if I update a definition twice <cough#1758cough> before completing my todos?
@pchiusano to schedule a time to discuss this one
update should maybe warn you if it's not any updating anything - to prevent adding to a new namespace
Also if we have namespace blocks then it doesn't matter where you are in UCM.
Current design and some commentary is here: https://gist.github.com/pchiusano/548032b98d5d6b29cb4314421a231a42
Just discussing this with @aryairani and we came up with a few ideas to keep the default patch super clean:
- The default patch should only contain things that pertain to your in-progress refactorings. Once you're done with a refactor (there's nothing
todo), that default patch gets moved topatches._2022_01_10_someguid. - On
merge, it checks for patches inpatchesthat the target branch doesn't know about. If there are any, it combines them and then applies them. - Question: what happens if applying the dev patch doesn't go smoothly? Possibilities:
- Just add the definitions to
old, andtodowill check for dependencies onold - Create a
mergepatch. todowill look at bothpatchandmergepatches in the current namespacetodoshould suggest:delete.patch merge(if there's nothingtodowith respect tomergepatch)- or
todocould just delete it
- Just add the definitions to
#3504 maybe goes here