Looks good to me.

Thanks bob. Just to say that I care about your code proposals and sorry if this needed extra time.

There was still a very small margin of probability that the $committer sometime was an object that could be casted to string, and that before was allowed, and we should not introduce an accidental crash because of the well-known cool strictness of phutil_nonempty_string().

Basically, the DiffusionCommitRef.getCommitter is based on DiffusionCommitRef.formatUser() that may directly expose an object, for example, if DiffusionCommitRef allows to have an email that is not a string but an object like PhutilEmailAddress.

So, my doubt is: can we receive a DiffusionCommit with an email that is a PhutilEmailAddress, or a similar object?

My answer is: very probably this is not our problem 😎 The only usages of new DiffusionCommitRef are string based. Examples:

• DiffusionLowLevelCommitQuery creates DiffusionCommitRef with emails that are parsed with PhutilEmailAddress but then are extracted as string (as far as I can understand)

So I'm 99.98% sure that this is safe since we only receive NULL or a string. Anything alien will cause a nuclear implosion, but Phorge loves that.

If somebody will be affected by such nuclear implosion, we will evaluate that case as usual to be less strict than this. Thanks bob.

Ready to land!