Summary

Fix a bug with author information for rbt land.

Review Request #14770 — Created Jan. 9, 2026 and submitted Jan. 14, 2026, 6:42 p.m.

Information

Owner

maubin

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

Our BaseSCMClient.merge and BaseSCMClient.create_commit methods
type the author argument as PatchAuthor. However, in our land
command, we actually pass a UserItemResource which we get from
review_request.get_submitter(). This was made evident in a bug where
rbt land would emit a warning about the author's profile being private
even though it wasn't. This happened because we try to access
author.full_name, which doesn't exist on the user resource.

This change addresses that bug and improves our handling of the case
where author information isn't available. Now the author argument
for these methods can be None, indicating that the author information
isn't available. All the SCM clients have been updated to handle this
accordingly.

The Git and Mercurial clients actually already had a unit test for this
case, and one has been added for Jujutsu.

This change also adds typing to the land command.

Testing Done

Ran unit tests.
Saw that the bug I was hitting with rbt land went away.

I don't have Jujutsu or Mercurial set up on my system so I
actually don't know if their unit tests for this pass. If possible
could someone apply this patch and confirm that the unit tests pass.

Commits

Summary	ID
Fix a bug with author information for rbt land. Our `BaseSCMClient.merge` and `BaseSCMClient.create_commit` methods type the `author` argument as `PatchAuthor`. However, in our land command, we actually pass a `UserItemResource` which we get from `review_request.get_submitter()`. This was made evident in a bug where `rbt land` would emit a warning about the author's profile being private even though it wasn't. This happened because we use `author.full_name`, which doesn't exist on the user resource. This change addresses that bug and improves our handling of the case where author information isn't available. Now the `author` argument can be `None`, indicating that the author information isn't available. All the SCM clients have been updated to handle this accordingly. This change also adds typing to the land command.	28825c6e2f561e12fd395b6685ed9f92dbac7127

Summary

Fix a bug with author information for rbt land.

Our `BaseSCMClient.merge` and `BaseSCMClient.create_commit` methods type the `author` argument as `PatchAuthor`. However, in our land command, we actually pass a `UserItemResource` which we get from `review_request.get_submitter()`. This was made evident in a bug where `rbt land` would emit a warning about the author's profile being private even though it wasn't. This happened because we use `author.full_name`, which doesn't exist on the user resource. This change addresses that bug and improves our handling of the case where author information isn't available. Now the `author` argument can be `None`, indicating that the author information isn't available. All the SCM clients have been updated to handle this accordingly. This change also adds typing to the land command.

28825c6e2f561e12fd395b6685ed9f92dbac7127

Issues

Description	From	Last Updated
Can you update this to be rbtools.diffs.patches.PatchAuthor?	david	Jan. 11, 2026, 6:21 p.m.
We don't need the parens around the type here.	david	Jan. 11, 2026, 6:22 p.m.
Shouldn't this be passing author=None?	david	Jan. 11, 2026, 6:22 p.m.
By default create_commit will call jj new at the end. I think instead of assertSpyLastCalledWith we should just use assertSpyCalledWith	david	Jan. 12, 2026, 11:17 a.m.
Same on this one.	david	Jan. 12, 2026, 11:17 a.m.

flake8 passed.

JSHint passed.

rbtools/clients/base/scmclient.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can you update this to be rbtools.diffs.patches.PatchAuthor?
```
rbtools/clients/git.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We don't need the parens around the type here.
```
rbtools/clients/tests/test_jujutsu.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Shouldn't this be passing author=None?
```
1. maubin Jan. 11, 2026, 6:27 p.m.
  Oops yep.

Commits:

	Summary	ID
	Fix a bug with author information for rbt land. Our `BaseSCMClient.merge` and `BaseSCMClient.create_commit` methods type the `author` argument as `PatchAuthor`. However, in our land command, we actually pass a `UserItemResource` which we get from `review_request.get_submitter()`. This was made evident in a bug where `rbt land` would emit a warning about the author's profile being private even though it wasn't. This happened because we use `author.full_name`, which doesn't exist on the user resource. This change addresses that bug and improves our handling of the case where author information isn't available. Now the `author` argument can be `None`, indicating that the author information isn't available. All the SCM clients have been updated to handle this accordingly. This change also adds typing to the land command.	42cfa07f8e8156d6868caa390ebc7741c8c0ed84
	Fix a bug with author information for rbt land. Our `BaseSCMClient.merge` and `BaseSCMClient.create_commit` methods type the `author` argument as `PatchAuthor`. However, in our land command, we actually pass a `UserItemResource` which we get from `review_request.get_submitter()`. This was made evident in a bug where `rbt land` would emit a warning about the author's profile being private even though it wasn't. This happened because we use `author.full_name`, which doesn't exist on the user resource. This change addresses that bug and improves our handling of the case where author information isn't available. Now the `author` argument can be `None`, indicating that the author information isn't available. All the SCM clients have been updated to handle this accordingly. This change also adds typing to the land command.	ffc7cda82ab85bdee5b97d6f33a4b173cb102b5d

Diff:

Revision 2 (+390 -92)

Show changes

	rbtools/clients/git.py
	rbtools/clients/jujutsu.py
	rbtools/clients/mercurial.py
	rbtools/clients/base/scmclient.py
	rbtools/clients/tests/test_jujutsu.py
	rbtools/commands/land.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ran the hg and jj tests. Two failing tests, with an easy fix:

maubin Jan. 12, 2026, 11:19 a.m.
```
Thanks for checking.
```

rbtools/clients/tests/test_jujutsu.py (Diff revision 2)

The issue has been resolved. Show all issues

By default create_commit will call jj new at the end. I think instead of assertSpyLastCalledWith we should just use assertSpyCalledWith

rbtools/clients/tests/test_jujutsu.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Same on this one.
```

Commits:

	Summary	ID
	Fix a bug with author information for rbt land. Our `BaseSCMClient.merge` and `BaseSCMClient.create_commit` methods type the `author` argument as `PatchAuthor`. However, in our land command, we actually pass a `UserItemResource` which we get from `review_request.get_submitter()`. This was made evident in a bug where `rbt land` would emit a warning about the author's profile being private even though it wasn't. This happened because we use `author.full_name`, which doesn't exist on the user resource. This change addresses that bug and improves our handling of the case where author information isn't available. Now the `author` argument can be `None`, indicating that the author information isn't available. All the SCM clients have been updated to handle this accordingly. This change also adds typing to the land command.	ffc7cda82ab85bdee5b97d6f33a4b173cb102b5d
	Fix a bug with author information for rbt land. Our `BaseSCMClient.merge` and `BaseSCMClient.create_commit` methods type the `author` argument as `PatchAuthor`. However, in our land command, we actually pass a `UserItemResource` which we get from `review_request.get_submitter()`. This was made evident in a bug where `rbt land` would emit a warning about the author's profile being private even though it wasn't. This happened because we use `author.full_name`, which doesn't exist on the user resource. This change addresses that bug and improves our handling of the case where author information isn't available. Now the `author` argument can be `None`, indicating that the author information isn't available. All the SCM clients have been updated to handle this accordingly. This change also adds typing to the land command.	28825c6e2f561e12fd395b6685ed9f92dbac7127

Diff:

Revision 3 (+390 -92)

Show changes

	rbtools/clients/git.py
	rbtools/clients/jujutsu.py
	rbtools/clients/mercurial.py
	rbtools/clients/base/scmclient.py
	rbtools/clients/tests/test_jujutsu.py
	rbtools/commands/land.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (f666029)