Skip to content
Snippets Groups Projects

gca refactoring

Merged Raphaël Gomès requested to merge topic/default/gca-refactoring into branch/default

This MR decouples the content-divergence resolution logic from gca which was unnecessarily populated here, as the same goal could be achieved by a simple 3-way comparison.

Edited by Raphaël Gomès

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • added 1 commit

    • 0f301a96e459 - evolve: separate the gca-logic from main resolution part

    Compare with previous version

    By Sushil Khanchi on 2021-01-10T06:25:06 (imported from GitLab)

  • Raphaël Gomès resolved all threads

    resolved all threads

    By Sushil Khanchi on 2021-01-10T06:48:42 (imported from GitLab)

  • Raphaël Gomès added 13 commits

    added 13 commits

    Compare with previous version

    By Pierre-Yves David on 2021-01-16T02:43:31 (imported from GitLab)

    • Resolved by Raphaël Gomès

      evolve: no need to keep it as a smartset as it's a single value

      Note that gca can be multiple revisions, this happens during criss cross merge. In practice after we reduces GCA usage to a single changesets, we mostly want to know if X in parents(ancestors(Y) - Y) and we use that :-D

      evolve: separate the gca-logic from main resolution part

      oof, the test change are quite brutal. I'll need a bit more time to review them. Could we have the refactoring without the logic change? It would make me more more comfortable.

      By Pierre-Yves David on 2021-01-18T08:59:16 (imported from GitLab)

  • Raphaël Gomès added 2 commits

    added 2 commits

    • fd7dfac95d94 - evolve: return rev from _pickresolutionparent()
    • 1f911f4d32dd - evolve: separate the gca-logic from main resolution part

    Compare with previous version

    By Sushil Khanchi on 2021-01-18T04:49:10 (imported from GitLab)

  • Raphaël Gomès added 15 commits

    added 15 commits

    Compare with previous version

    By Pierre-Yves David on 2021-01-22T04:28:02 (imported from GitLab)

  • Raphaël Gomès added 5 commits

    added 5 commits

    Compare with previous version

    By Pierre-Yves David on 2021-01-22T04:41:11 (imported from GitLab)

    • Resolved by Raphaël Gomès

      I started doing work for splitting this is more diguestable bits.

      I suspect everything up to:

      divergence-resolution: deal with simple "just one moved case" earlier

      Is in a good enough shape to get in. @khanchi97 can you double check them ?

      The rest of the series is a bit muckier. I initially had no idea what the extra change to test-evolve-content-divergent-interrupted.t were about. However I later got a better grasp on them. It seems that the order change mean we no longer have a conflict and that the test is about testing conflict. We probably need more explanation about that part.

      In addition the change around elif succsbasep1 in (divp1, otherp1): is a bit weird. So we should probably clarify it before going further in the test modificiation.

      I understand the logic but:

      • why do we only do it the base ?
      • shouldn't we just consider "single-successors" for everything anyway ? for the first logic ?

      If in this code:

          elif basep1 in (divp1, otherp1):
              # only one side moved, set parent of moved one as resolution parent
              if basep1 == divp1:
                  resolutionparent = succsotherp1
              else:
                  pass

      We would replace all of basep1, divp1 and otherp1 by their "single-successors", we would have the upgraded logic that we are trying to do with that change, don't we ?

      By Pierre-Yves David on 2021-01-23T04:37:01 (imported from GitLab)

  • Raphaël Gomès assigned to @khanchi97 and unassigned @marmoute

    assigned to @khanchi97 and unassigned @marmoute

    By Pierre-Yves David on 2021-01-22T09:50:00 (imported from GitLab)

  • Raphaël Gomès added 5 commits

    added 5 commits

    Compare with previous version

    By Sushil Khanchi on 2021-01-23T04:36:30 (imported from GitLab)

  • Raphaël Gomès added 2 commits

    added 2 commits

    • 6d93922432ff - FOLLOW SUCCESSORS, FIX ME ?
    • 5f65b3dd3cb4 - FIX ME: evolve: separate the gca-logic from main resolution part

    Compare with previous version

    By Sushil Khanchi on 2021-01-23T04:38:23 (imported from GitLab)

  • @marmoute you can publish up to the revision

    divergence-resolution: deal with simple "just one moved case" earlier

    I am looking into the remaining commits to fix them accordingly.

    By Sushil Khanchi on 2021-01-23T04:40:37 (imported from GitLab)

  • Raphaël Gomès added 9 commits

    added 9 commits

    • deff2e70dd19 - divergence-resolution: return rev from _pickresolutionparent()
    • 551de82cbc53 - divergence-resolution: pre-indent some code
    • 8705ccb23fc8 - divergence-resolution: pass the base changeset to _pickresolutionparent
    • da73aa37f739 - divergence-resolution: clarify some test case
    • d32f8912061d - divergence-resolution: remove explicite hash in some command
    • 7a91d36ba310 - divergence-resolution: deal with simple "just one moved case" earlier
    • 70f443559c50 - divergence-resolution: [patch 1] fix a messed up test case
    • 522247b4d68f - divergence-resolution: [patch 2] fix a messed up test case
    • 846b3c8f1782 - divergence-resolution: [patch 3] fix a messed up test case

    Compare with previous version

    By Sushil Khanchi on 2021-01-24T18:04:03 (imported from GitLab)

  • Raphaël Gomès
  • Raphaël Gomès assigned to @marmoute and unassigned @khanchi97

    assigned to @marmoute and unassigned @khanchi97

    By Sushil Khanchi on 2021-01-25T03:39:27 (imported from GitLab)

  • Raphaël Gomès added 30 commits

    added 30 commits

    • 846b3c8f...61d2f1cf - 20 commits from branch branch/default
    • e6b0ed34bc18 - divergence-resolution: return rev from _pickresolutionparent()
    • 1319abf8d8c4 - divergence-resolution: pre-indent some code
    • bc323205241b - divergence-resolution: pass the base changeset to _pickresolutionparent
    • 6dbfd71cdcba - divergence-resolution: clarify some test case
    • 3a1e07c27e4a - divergence-resolution: remove explicite hash in some command
    • a119432bafc1 - divergence-resolution: deal with simple "just one moved case" earlier
    • 04095b70bee5 - divergence-resolution: take parent successors in account
    • caee3020a5c5 - divergence-resolution: [patch 1] fix a messed up test case
    • 0d3c452e8a9d - divergence-resolution: [patch 2] fix a messed up test case
    • 9decfbec933e - divergence-resolution: [patch 3] fix a messed up test case

    Compare with previous version

    By Pierre-Yves David on 2021-01-25T13:30:10 (imported from GitLab)

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading