Closed Bug 1487997 Opened 2 years ago Closed 2 years ago

webdriver: Fix needless string conversions and formattings

Categories

(Testing :: geckodriver, enhancement)

enhancement
Not set

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: ato, Assigned: ato)

Details

Attachments

(1 file)

We use format!() sometimes unnecessarily and there’s some identical conversions.
Assignee: nobody → ato
Status: NEW → ASSIGNED
Also fixes some x.or() to x.or_else() so that the closures are not
evaluated unless they have to be.
Attachment #9005827 - Flags: review?(hskupin)
Comment on attachment 9005827 [details] [diff] [review]
webdriver: fix needless string formatting

Review of attachment 9005827 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/webdriver/src/command.rs
@@ +520,5 @@
>  fn deserialize_to_u64<'de, D>(deserializer: D) -> Result<Option<u64>, D::Error>
>  where
>      D: Deserializer<'de>,
>  {
> +    let opt = Option::deserialize(deserializer)?.map(|x: f64| x);

What's the benefit of changing this line? It only makes it harder to blame code.
Attachment #9005827 - Flags: review?(hskupin) → review+
(In reply to Henrik Skupin (:whimboo) from comment #2)

> ::: testing/webdriver/src/command.rs
> @@ +520,5 @@
> >  fn deserialize_to_u64<'de, D>(deserializer: D) -> Result<Option<u64>, D::Error>
> >  where
> >      D: Deserializer<'de>,
> >  {
> > +    let opt = Option::deserialize(deserializer)?.map(|x: f64| x);
> 
> What's the benefit of changing this line? It only makes it harder to blame
> code.

I just found it confusing that there were two variables after one
another named ‘value’, but I can revert this.
Pushed by ato@sny.no:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7f1cad8ce39
webdriver: fix needless string formatting; r=whimboo
https://hg.mozilla.org/mozilla-central/rev/a7f1cad8ce39
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.