Bug 106584 – Dashboard Cleanup: Do not generate the page if we are about to reload.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Julie Parent]

Dashboard Cleanup: Do not generate the page if we are about to reload.

[Julie Parent]

Created attachment 182173 [details]
Patch

[Dirk Pranke]

Comment on attachment 182173 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182173&action=review

> Tools/TestResultServer/static-dashboards/dashboard_base.js:-330
> -                return {};

can you add a comment up at line 318 indicating that the function returns whether to generate the page or not? The function name is slightly misleading since it's now parsing parameters *and* telling you whether to generate the page, but I can't think of a better function name that would capture that w/o being awkward.

[Ojan Vafai]

Comment on attachment 182173 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182173&action=review

> Tools/TestResultServer/static-dashboards/dashboard_base.js:353
> +        shouldGeneratePage = handleQueryParameterChange(dashboardSpecificDiffState);

Nit: early return here and you can get rid of the local variable.

[Dirk Pranke]

(In reply to comment #3)
> (From update of attachment 182173 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182173&action=review
> 
> > Tools/TestResultServer/static-dashboards/dashboard_base.js:353
> > +        shouldGeneratePage = handleQueryParameterChange(dashboardSpecificDiffState);
> 
> Nit: early return here and you can get rid of the local variable.

I thought of that too but realized that it's not clear what the true/false indicates, and so a named variable is helpful (cf. my other comment).

[Julie Parent]

Created attachment 182179 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 182179 [details]
Patch for landing

Clearing flags on attachment: 182179

Committed r139348: <http://trac.webkit.org/changeset/139348>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.