This is an archive of the discontinued Mercurial Phabricator instance.

dirstate: fix some typos in docstrings
ClosedPublic

Authored by Alphare on Apr 6 2022, 10:10 AM.

Details

Summary

I was passing by and they've been bothering me. :)

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Apr 6 2022, 10:10 AM
marmoute accepted this revision.Apr 8 2022, 6:36 AM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.

@martinvonz I've been amending my series since @marmoute suggested changes, it should not have been pushed. I'm not sure what I'm supposed to do now, the rebase is going to be really painful. I haven't pulled from committed yet.

@martinvonz I've been amending my series since @marmoute suggested changes, it should not have been pushed. I'm not sure what I'm supposed to do now, the rebase is going to be really painful. I haven't pulled from committed yet.

Hmm, I thought the series was all green in Yadda. Feel free to prune the version in hg-committed.

baymax updated this revision to Diff 32915.Apr 8 2022, 12:29 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

@martinvonz I've been amending my series since @marmoute suggested changes, it should not have been pushed. I'm not sure what I'm supposed to do now, the rebase is going to be really painful. I haven't pulled from committed yet.

Hmm, I thought the series was all green in Yadda. Feel free to prune the version in hg-committed.

Or, if it's easier for you, I can strip the commits and force-push the @ bookmark backwards. Let me know if you prefer that. Sorry I missed the comments in the series (I generally queue rust code without looking, but I'll try to at least look for review comments in the future).

Or, if it's easier for you, I can strip the commits and force-push the @ bookmark backwards. Let me know if you prefer that.

I've fixed the issue. Some changesets got published, my topics got pushed accidentally for some unknown reason and other fun things. I am *so* done with Phabricator, I cannot express how much I want the entire review process overhauled... but that's for the discussion on the ML.

Sorry I missed the comments in the series (I generally queue rust code without looking, but I'll try to at least look for review comments in the future).

I am not sure how to read this. The code being written in Rust has little bearing with its need for review? I can and have sent broken or sub-optimal Rust code before on multiple occasions, a lot of which was caught in review, not by the compiler. Please review the Rust series as well?

Or, if it's easier for you, I can strip the commits and force-push the @ bookmark backwards. Let me know if you prefer that.

I've fixed the issue. Some changesets got published, my topics got pushed accidentally for some unknown reason and other fun things. I am *so* done with Phabricator, I cannot express how much I want the entire review process overhauled... but that's for the discussion on the ML.

According to the logs, those commits were stamped by both you and me:

3abf799b rgomes alphare martinvonz
dirstate: fix some typos in docstrings
8309c83b rgomes alphare martinvonz
test-issue660: test inside a repository, not the test dir
x524819ab rgomes alphare martinvonz
test-issue660: add dirstate-v2 variant
x362312d6 rgomes alphare martinvonz
rust-dirstate-entry: fix typo in panic message
be9bf75a rgomes alphare martinvonz
dirstate: remove v1_* methods from Python/C/Rust shared API
a85c123c rgomes @committed alphare martinvonz
rust-dirstate: don't return a state for untracked entries

The script that runs on mercurial-scm.org checks for unchanged diffs, so I think what happened is that I pushed your commits, which implicitly marks them accepted by me. Then you rebased them and pushed them, which resulted in you accepting them as well. In the cases where the diffs were unchanged, the script would make them public. I don't think Phabricator is involved in the pushing or publishing.

Sorry I missed the comments in the series (I generally queue rust code without looking, but I'll try to at least look for review comments in the future).

I am not sure how to read this. The code being written in Rust has little bearing with its need for review? I can and have sent broken or sub-optimal Rust code before on multiple occasions, a lot of which was caught in review, not by the compiler. Please review the Rust series as well?

I got the impression a long time ago that the Rust code was an experiment by Octobus that I shouldn't slow down by reviewing and commenting. Since it's experimental and not enabled by default, that seemed fine to me. Sorry if I misunderstood. I'll review Rust code as well from now on.

The script that runs on mercurial-scm.org checks for unchanged diffs, so I think what happened is that I pushed your commits, which implicitly marks them accepted by me. Then you rebased them and pushed them, which resulted in you accepting them as well. In the cases where the diffs were unchanged, the script would make them public.

Right, I thought this was something to do with this. This adds more fuel to my fire about the review tooling. :D

I don't think Phabricator is involved in the pushing or publishing.

I was specifically targeting Phab because it's the current default review tool, it doesn't understand obsolescence and is out-of-band, but it's the entire review system that needs an overhaul.

I got the impression a long time ago that the Rust code was an experiment by Octobus that I shouldn't slow down by reviewing and commenting. Since it's experimental and not enabled by default, that seemed fine to me. Sorry if I misunderstood. I'll review Rust code as well from now on.

I have always received some amount of review on the Rust code, I'm not sure if this was ever the case, but maybe before my time in Mercurial. Thanks for confirming then. I certainly appreciate the sentiment of "I shouldn't slow down by reviewing and commenting", and trusting some people with experience but without reviewer rights (like marmoute or Simon formerly) can be a valid way of speeding it up IMO, but the code is now being used by a fair amount of people, so it still needs review by *somebody*.