| martinvonz |
| hg-reviewers |
This method is only about whether there are file changes, not about whether the
commit will be empty or not.
One user of the method was incorrectly assuming the latter meaning, leading to
the bug for which a test case was added in D8727. I’ve added a FIXME to the
code.
The original motivation for the rename was that I want to add
committablectx.isempty(), that properly checks if a commit will be empty,
using the exact same logic as in repo.commit(), and I wanted to avoid a name
clash.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| mercurial/context.py | ||
|---|---|---|
| 2519–2529 | I feel like it would be better to find better names for these three methods together since they're about the same concept. Maybe simply renaming this one to isclean() matches well? The only issue I see is that Mercurial usually skips the "is" prefix and "clean" is both a verb and an adjective, so it's not obvious at first why there's both clean() and isclean() (people familiar with Mercurial naming might think they're the same). Probably still okay? | |
| mercurial/context.py | ||
|---|---|---|
| 2519–2529 | To be honest, I didn’t think too hard about this name (at long as it’s more correct as the old name), as the method will become unused and removed a few patches later. If you still think we should rename it for the in-between steps, I’d be fine renaming it. | |
| mercurial/context.py | ||
|---|---|---|
| 2519–2529 | No, that's fine. I had just not realized that it was going away in a later patch. | |