This moves cmdutil.unfinishedstates, checkunfinished(),clearunfinished()
to state.py. the already existing users of this module are updated accordingly.
Test results remain unchanged.
Thanks for working on this!
I think this patch does too much at once. At least the help message change should be moved to a separate patch. Ideally, the other changes would also be split up. Perhaps something like this:
You don't have to follow those steps exactly, of course. You know the code better, so maybe some of what I suggested doesn't make sense.
Pass at least the boolean arguments as named arguments (clearable=False etc)
@martinvonz I have completed steps 1 to 5 as you stated in the sequence above in statecheck.py. For the purpose of showing that it works without flaw, I redirected the API calls to the new API for both STATES and unfinished state and removed those from cmdutil.py. All that remains is a minor bug that I am facing with hg bisect. I will clear that soon enough. Do you want me to send the API integration and tests as a separate patch and API as another?
Thanks for splitting this out! Sorry about being so particular about splitting up the patch. I think it's an important part of writing patches that others will review. Hopefully it wasn't just frustrating and you can also see some benefit with it. The benefit is mostly to reviewers and future code archeologists and not to the author, so it can feel like wasting time. Remember that more people will read the patches than the people writing them (which is just you).
|1015 ↗||(On Diff #15586)|
I wonder if we should keep the functions in cmdutil.py and let them delegate to the new ones. That way we won't have to update all the callers. I think cmdutil sounds like a more natural place for the callers to looks for this function. That's also where bailifchanged() is, so it it makes sense that these two similar functions are in the same place.
|35–49 ↗||(On Diff #15586)|
Delete (looks like a bad rebase)