Page MenuHomeWildfire Games

Linting: Enable 'no-floating-decimal' rule in ESLint
ClosedPublic

Authored by Krinkle on Jun 20 2019, 1:14 AM.

Details

Reviewers
elexis
historic_bruno
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22472: Enable eslint rule no-floating-decimal and remove all superfluous floating…
Trac Tickets
#5524
Summary

This matches the W008 and W047 warnings that JSHint already emits for us.
(As seen at https://code.wildfiregames.com/D1993#83345).

The equivalent rule in ESLint is no-floating-decimal. Enable this so developers can
detect this while writing code in IDEs with an ESLint plugin (which is more widely
available than the now-lesser maintainer JSHint, and more contributors will have
this installed already).

It also means we can auto-fix them in the future with eslint --fix.

This commit fixes the two violations that existed in the code base. Thus, enabling this
won't add any Vulcan noise.

https://eslint.org/docs/rules/no-floating-decimal

Test Plan

Happy Coala bears.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Krinkle created this revision.Jun 20 2019, 1:14 AM
Harbormaster completed remote builds in B8033: Diff 8567.
Krinkle edited the summary of this revision. (Show Details)Fri, Jun 21, 9:55 PM
Krinkle edited the summary of this revision. (Show Details)Sat, Jul 13, 11:09 PM
Krinkle updated this revision to Diff 8871.Sat, Jul 13, 11:16 PM
Krinkle edited the summary of this revision. (Show Details)
Krinkle edited the test plan for this revision. (Show Details)
Krinkle added reviewers: elexis, historic_bruno.
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Sat, Jul 13, 11:16 PM
Krinkle added inline comments.Sat, Jul 13, 11:17 PM
binaries/data/mods/public/simulation/ai/petra/headquarters.js
529 ↗(On Diff #8871)

Intentional, to see that it gets spotted by Coala. Will fix in a minute.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/60/display/redirect

Krinkle updated this revision to Diff 8873.Sat, Jul 13, 11:26 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/62/display/redirect

Krinkle updated this revision to Diff 8874.Sat, Jul 13, 11:39 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/63/display/redirect

I see no problem with this, but today is my first encounter with this aspect of the build, so I'll need a better grasp before reviewing.

Krinkle updated this revision to Diff 8877.Sun, Jul 14, 1:22 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/66/display/redirect

elexis accepted this revision.EditedSun, Jul 14, 11:58 AM

Correctness:
I agree that having this rule is nice to improve consistency, and in my practice since I'm here we always removed trailing zeroes and 'trailing periods'.
Then read the specs here https://eslint.org/docs/rules/no-floating-decimal.

Completeness:
From IRC yesterday:

23:28 < elexis> how did you find out its the only occurrences?
23:28 < elexis> eslint commandline tool?
23:29 < Krinkle> Yes

which is enough for me to believe that to be the case until further evidence.

Then I wrote this and noticed that I don't like the word belief, so I installed eslint, ran eslint -c build/jenkins/lint-config/eslintrc.json .
Then my system froze because I did too many things at the same time.
Then I rebooted, tried again and got a JavaScript heap out of memory.
Then I called eslint -c build/jenkins/lint-config/eslintrc.json binaries/data/mods/ | grep "no-floating-decimal" without the headquarters diff, got 1 match, applied the headquarters diff, got 0 results, confirming that it works as intended and that the patch is complete.
Thanks for the patch.

23:27 < elexis> Krinkle: mind if I commit without that zero?
23:27 < Krinkle> you mean 0.70?
23:27 < Krinkle> Yeah, that's fine to change I guess.

This revision is now accepted and ready to land.Sun, Jul 14, 11:58 AM
This revision was landed with ongoing or failed builds.Sun, Jul 14, 12:03 PM
This revision was automatically updated to reflect the committed changes.
Krinkle updated the Trac tickets for this revision.Sat, Jul 20, 9:33 PM