[darcs-devel] How to merge rebase refactoring

Ganesh Sittampalam ganesh at earth.li
Fri Feb 19 18:57:22 UTC 2016


That's no problem, once it's in screened I'm not too worried about the
lag to actually review it.

On 19/02/2016 18:43, Guillaume Hoffmann wrote:
> I like solution #2 (one big patch) more than #1 since anyway I would
> review all patches at once anyway (if I happen to be the reviewer).
> Just so you know, I'm not going to be able to look at it until the 2nd
> of march.
> 
> Guillaume
> 
> 2016-02-19 4:26 GMT-03:00 Ganesh Sittampalam <ganesh at earth.li>:
>> Hi,
>>
>> The new "stash" feature is still some way from merging as the UI needs
>> sorting out. In the meantime I'm hoping to get the major refactoring to
>> rebase that it depends on merged, to reduce future conflicts.
>>
>> I've put up a draft here:
>>
>> http://hub.darcs.net/ganesh/darcs-rebase-refactor-20160218/compare/darcs/darcs-screened
>>
>> There's a chunk of patches in the middle that are split into logical
>> parts, but can't be applied without each other - the intermediate states
>> have missing cases etc and would fail at runtime:
>>
>> - add RebaseP
>> - introduce RebaseType
>> - replace MaybeInternal with IsRepoType
>> - fix fmapFL_WrappedNamed
>> - get rid of Rebasing
>> - read rebase patches into RebaseP
>> - sort out Commute/Merge instances
>>
>> When I was originally developing the refactoring I introduced a
>> temporary type class ("MatchRebasing") that I used to keep the code
>> compiling at each stage, but that isn't included in the patches above.
>> Having it involved adding the type class temporarily to various
>> constraints all over the codebase and then later removing it again.
>>
>> So, I see three options for submitting the patches above:
>>
>>  - Submit roughly as they are now. Easy for me to do, but this doesn't
>> feel like a good option given the intermediate broken states in the repo
>> that would obstruct tracking down problems in future.
>>
>>  - Merge them all into one patch and submit that. Harder to review and
>> to understand and harder in future to understand the history, but also
>> easy for me to do.
>>
>>  - Update them to use the "MatchRebasing" temporary typeclass again.
>> More work for me, and introduces a weird temporary thing that will
>> clutter history a bit, but keeps each patch individually reviewable and
>> understandable in the history.
>>
>> Please let me know what you think would be best.
>>
>> Further background on the changes:
>>
>> The essence of the changes is a shift in the internal representation of
>> repositories with a rebase in progress - the type changes from:
>>
>>     Named (Rebasing p)
>>
>> to
>>
>>     WrappedNamed ('RepoType 'IsRebase) p
>>
>> WrappedNamed is a new type that goes around Named:
>>
>>     data WrappedNamed (rt :: RepoType) p wX wY where
>>         NormalP :: !(Named p wX wY) -> WrappedNamed rt p wX wY
>>         RebaseP
>>           :: (PrimPatchBase p, Show2 (PrimOf p), FromPrim p, Effect p)
>>           => !PatchInfo
>>           -> !(Rebase.Suspended p wX wX)
>>           -> WrappedNamed ('RepoType 'IsRebase) p wX wX
>>
>> So where before the "rebase container" patch would have been
>>
>>     NamedP pi (Suspended s :>: NilFL)
>>
>> now it's
>>
>>     RebaseP pi s
>>
>> and where normal patches would have been
>>
>>    NamedP pi (Normal p)
>>
>> they now become
>>
>>    NormalP (NamedP pi p)
>>
>> The rebase container patch is still a hack, but now in the code it lives
>> closer to where it should. Ideally it would just live at the Repository
>> level rather than the Patch level, but that's for another day.
>>
>> The sequence of patches above starts from the introduction of the
>> RebaseP constructor in WrappedNamed, and finishes when that constructor
>> is properly handled everywhere in the code.
>>
>> Cheers,
>>
>> Ganesh
>> _______________________________________________
>> darcs-devel mailing list
>> darcs-devel at darcs.net
>> http://lists.osuosl.org/mailman/listinfo/darcs-devel
> _______________________________________________
> darcs-devel mailing list
> darcs-devel at darcs.net
> http://lists.osuosl.org/mailman/listinfo/darcs-devel
> 



More information about the darcs-devel mailing list