Details
- Reviewers
baymax - Group Reviewers
hg-reviewers
Diff Detail
- Repository
- rHG Mercurial
- Branch
- default
- Lint
No Linters Available - Unit
No Unit Test Coverage
Event Timeline
While it's still POC, having some commit message on how we are tackling the problem will be helpful.
The proof of concept part is primarily that we can get the benefit with minimal refactoring of the existing rbc implementation. The actual use is a bit more complicated as the actual topicmap would be part of an extension and therefore hook up differently. But the interaction is much easier to understand like this, IMO.
I like where this is headed. Thank you for doing this work. I can see this patch can be split into at least 3 different patches which will help speed up review.
mercurial/branchmap.py | ||
---|---|---|
547 | This movement of variables can be a separate patch and will be easy one to queue making this patch a bit lighter to review. | |
782 | Probably we can have an interface for generic name-cache. | |
mercurial/changelog.py | ||
599 | This and related changes should be a separate patch. |
... of course when it's not a POC anymore.
mercurial/branchmap.py | ||
---|---|---|
599 | Also, what does a caller passing extra as None is expecting here? It will be nice to not have this an optional argument. We can also pass a rev/node here and do changelog.changelogrevision thing in this function. |
mercurial/branchmap.py | ||
---|---|---|
547 | Right, the primary goal of this PoC is to outline what changes are necessary and what the impact is for supporting more than just the branch mapping. It doesn't make sense as is since it is provided for the benefit out of currently out-of-tree target. So the goal is to get a consensus on whether the cache is useful enough for the topic extension and if it is, split this patch into smaller pieces, figure out a few details in the method naming (e.g. branchinfo is tight to the current functionality) in core and how to integrate the rest backwards compatible in the topic extension. | |
599 | As mentioned in the review for the parent, doing a lookup here is expensive and should be avoided. The extra=None case is essentially only used for nullrev. The function name is not terribly good, parsechangeset or so would make a lot more sense in the generalized cache. | |
mercurial/changelog.py | ||
599 | Yes, but it is a breaking change for topic. |
There seems to have been no activities on this Diff for the past 3 Months.
By policy, we are automatically moving it out of the need-review state.
Please, move it back to need-review without hesitation if this diff should still be discussed.
:baymax:need-review-idle:
This movement of variables can be a separate patch and will be easy one to queue making this patch a bit lighter to review.