1310618 - Replace |res| for nsreult under editor/ with |rv|

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

In our coding rules, variable name for nsresult should be |rv|.  However, a lot of places under editor/ still use |res|.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6629db4b093

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6ea24371292

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c2ee0741dc1

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Bug 1310618 Replace nsresult variables |res| with |rv| under editor/

In our coding rules, variable names of nsresult should be rv.  Indeed, when you see |rv| in the code, you must assume that its type if nsresult.

However, a lot of code under editor/ uses |res| for the variables of nsresult.  Let's replace |res| with |rv|.

And this patch improves following points:

1. When |rv| is set in both |if| and |else| block and they are check outside of them, this moves the check into each |if| and |else| block because even if the failure is notified with warning, you cannot see which case was performed and failed.  This change makes it clear.

2. When |return rv;| returns non-error code because |rv| is checked with NS_ENSURE_SUCCESS() immediately before, setting replacing it with |return NS_OK;| is clearer.

3. Move declaration of |nsresult rv| into smaller scope as far as possible.  This prevents setting rv to unexpected value and easier to check its value at reading the code.

Review commit: https://reviewboard.mozilla.org/r/87096/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/87096/

Comment 5

[(no longer active)]

I'm really trying not to review editor/ code any more.  Can you please find someone else who's willing to review this stuff in the future?  Thanks in advance.

Comment 6

[(no longer active)]

Comment on attachment 8802795 [details]
Bug 1310618 Replace nsresult variables |res| with |rv| under editor/

https://reviewboard.mozilla.org/r/87096/#review86264

rs+

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

(In reply to :Ehsan Akhgari (Away Oct 25 - Nov 9) from comment #5)
> I'm really trying not to review editor/ code any more.  Can you please find
> someone else who's willing to review this stuff in the future?  Thanks in
> advance.

Okay, thank you for your review for them!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae369aab8479
Replace nsresult variables |res| with |rv| under editor/ r=Ehsan

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ae369aab8479