This is an archive of the discontinued Mercurial Phabricator instance.

infinitepush: remove duplicated logic for storing changegroups
Needs RevisionPublic

Authored by pulkit on Dec 5 2017, 2:11 PM.
Tags
None
Subscribers
None

Details

Reviewers
durham
Group Reviewers
Restricted Project
Summary

Before this patch, we had same code in parthandler for scratchbranchparttype and
processpart where we override bundle2.processpart and add storing changegroup
logic if we get scratchbranchparttype.

Since we have all this logic in parthandler, we can just go ahead and process
the scratchbranchparttype instead of adding it in processparts().

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Dec 5 2017, 2:11 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 5 2017, 2:11 PM

All the test-infinitepush-* skips for me except one, so I am not sure whether this breaks something or not. Any help on how can I run the test locally will be of great help.

durham requested changes to this revision.Dec 7 2017, 4:59 PM
durham added inline comments.
infinitepush/__init__.py
1125

If we delete this code, when does this bundler get processed? I think the purpose of having this serialization logic is so that every other part in the bundle gets processed above first, and then we serialize it here. Without this code here, then the bundler is just thrown away and all the parts that were added to it are lost, and the only serialization ends up being from the scratchbranchparttype processing, which doesn't handle the other parts.

Or am I forgetting something?

This revision now requires changes to proceed.Dec 7 2017, 4:59 PM
pulkit added inline comments.Dec 7 2017, 5:20 PM
infinitepush/__init__.py
1125

There is part handler defined for that already which has exact same logic which I just deleted. The only difference I see is that, in this case, we are storing changegroup in the end, while that won't be executed in the end.

1254

@durham I am referring to this part handler part. It has the same logic which I deleted above.

durham added inline comments.Dec 7 2017, 7:44 PM
infinitepush/__init__.py
1125

This one is storing the entire bundler though. So every part that was added to the bundler above is in the infinitepush stored bundle. The standard scratchbranchparttype part handler would result in a bundle that only contained the changegroup part. So the new code allows us to produce a bundle that contain parts like obsmarkers and stuff.

pulkit added inline comments.Dec 7 2017, 7:56 PM
infinitepush/__init__.py
1125

Ah, thanks for the explanation. I got confused by the exact same code and variable names.