[StyleGuide] Allow const in JavaScript code

[StyleGuide] Allow const in JavaScript code

This CL allows use of the const keyword in JavaScript code, after offline
discussion with benjhayden@ and charliea@.

BUG=catapult:#2833

Review-Url: https://codereview.chromium.org/2466263003
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/3117a26ce459be98f34a94cf0437ad5e300c21e3

[eakuefner, 2016-11-01 17:56:12]

eakuefner@chromium.org changed reviewers:
+ benjhayden@chromium.org, charliea@chromium.org

[eakuefner, 2016-11-01 17:56:12]

PTAL. One thing I'm wondering about is whether it actually makes sense to hold
adoption of `let` -- it may be that we should just allow it in this CL together
with `const`. There's a concern about consistency but I think it's the same
concern as for `const` or, say, classes, so I think my vote might actually be to
allow it. Both keywords provide productivity improvements for developers.

[benjhayden, 2016-11-01 18:09:22]

On 2016/11/01 at 17:56:12, eakuefner wrote:
> PTAL. One thing I'm wondering about is whether it actually makes sense to hold
adoption of `let` -- it may be that we should just allow it in this CL together
with `const`. There's a concern about consistency but I think it's the same
concern as for `const` or, say, classes, so I think my vote might actually be to
allow it. Both keywords provide productivity improvements for developers.

lgtm to allow both `const` and `let`, or just `const` if Charlie has a reason to
hold off on `let`.

[charliea (OOO until 10-5), 2016-11-01 19:32:37]

I think what's important here is that we have a strategy for what we want to
encourage people to do.

I think `const` is a little easier because there aren't as many constants, and
we can just tell people, "Hey, instead of var X_Y_Z = 0, use const X_Y_Z = 0;",
and that recommendation is pretty constrained. 

Are we allowing people to use `let` but not recommending it? Are we recommending
it for new code, but not recommending it as people touch old code? Are we
requiring conversion for any code that people are touching? I think my vote is
for the second option, but I can see arguments for a lot of them, and I think
it's important that we have a plan before we allow this.

[benjhayden, 2016-11-01 21:42:20]

On 2016/11/01 at 19:32:37, charliea wrote:
> I think what's important here is that we have a strategy for what we want to
encourage people to do.
> 
> I think `const` is a little easier because there aren't as many constants, and
we can just tell people, "Hey, instead of var X_Y_Z = 0, use const X_Y_Z = 0;",
and that recommendation is pretty constrained. 
> 
> Are we allowing people to use `let` but not recommending it? Are we
recommending it for new code, but not recommending it as people touch old code?
Are we requiring conversion for any code that people are touching? I think my
vote is for the second option, but I can see arguments for a lot of them, and I
think it's important that we have a plan before we allow this.

Should this cl remove ConstCheck?
https://github.com/catapult-project/catapult/blob/e1b701cbabeeac404049a83ffcc...

[benjhayden, 2016-12-14 19:31:50]

On 2016/11/01 at 21:42:20, benjhayden wrote:
> On 2016/11/01 at 19:32:37, charliea wrote:
> > I think what's important here is that we have a strategy for what we want to
encourage people to do.
> > 
> > I think `const` is a little easier because there aren't as many constants,
and we can just tell people, "Hey, instead of var X_Y_Z = 0, use const X_Y_Z =
0;", and that recommendation is pretty constrained. 
> > 
> > Are we allowing people to use `let` but not recommending it? Are we
recommending it for new code, but not recommending it as people touch old code?
Are we requiring conversion for any code that people are touching? I think my
vote is for the second option, but I can see arguments for a lot of them, and I
think it's important that we have a plan before we allow this.
> 
> Should this cl remove ConstCheck?
>
https://github.com/catapult-project/catapult/blob/e1b701cbabeeac404049a83ffcc...

ping :-)

[eakuefner, 2017-01-03 19:49:55]

On 2016/11/01 at 21:42:20, benjhayden wrote:
> On 2016/11/01 at 19:32:37, charliea wrote:
> > I think what's important here is that we have a strategy for what we want to
encourage people to do.
> > 
> > I think `const` is a little easier because there aren't as many constants,
and we can just tell people, "Hey, instead of var X_Y_Z = 0, use const X_Y_Z =
0;", and that recommendation is pretty constrained. 
> > 
> > Are we allowing people to use `let` but not recommending it? Are we
recommending it for new code, but not recommending it as people touch old code?
Are we requiring conversion for any code that people are touching? I think my
vote is for the second option, but I can see arguments for a lot of them, and I
think it's important that we have a plan before we allow this.
> 
> Should this cl remove ConstCheck?
>
https://github.com/catapult-project/catapult/blob/e1b701cbabeeac404049a83ffcc...

Yes. Coming back to this CL; will land by EOW.

[eakuefner, 2017-01-09 22:17:35]

Description was changed from

==========
[StyleGuide] Allow const in JavaScript code

This CL allows use of the const keyword in JavaScript code, after offline
discussion with benjhayden@ and charliea@.
==========

to

==========
[StyleGuide] Allow const in JavaScript code

This CL allows use of the const keyword in JavaScript code, after offline
discussion with benjhayden@ and charliea@.

BUG=catapult:#2833
==========

[eakuefner, 2017-01-09 22:21:27]

On 2016/11/01 at 21:42:20, benjhayden wrote:
> On 2016/11/01 at 19:32:37, charliea wrote:
> > I think what's important here is that we have a strategy for what we want to
encourage people to do.
> > 
> > I think `const` is a little easier because there aren't as many constants,
and we can just tell people, "Hey, instead of var X_Y_Z = 0, use const X_Y_Z =
0;", and that recommendation is pretty constrained. 
> > 
> > Are we allowing people to use `let` but not recommending it? Are we
recommending it for new code, but not recommending it as people touch old code?
Are we requiring conversion for any code that people are touching? I think my
vote is for the second option, but I can see arguments for a lot of them, and I
think it's important that we have a plan before we allow this.
> 
> Should this cl remove ConstCheck?
>
https://github.com/catapult-project/catapult/blob/e1b701cbabeeac404049a83ffcc...

Done.

[eakuefner, 2017-01-09 22:21:57]

I will continue the 'let' discussion in a followup CL.

[eakuefner, 2017-01-09 22:22:01]

The CQ bit was checked by eakuefner@chromium.org

[eakuefner, 2017-01-09 22:22:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
benjhayden@chromium.org
Link to the patchset: https://codereview.chromium.org/2466263003/#ps20001
(title: "shorten let wording, remove ConstCheck")

[commit-bot: I haz the power, 2017-01-09 22:22:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-09 22:24:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-09 22:24:14]

Try jobs failed on following builders:
  Catapult Presubmit on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Pr...)

[eakuefner, 2017-01-09 22:39:34]

eakuefner@chromium.org changed reviewers:
+ sullivan@chromium.org

[eakuefner, 2017-01-09 22:39:35]

+sullivan for catapult_build/js_checks.py

[sullivan, 2017-01-09 22:44:58]

lgtm

[eakuefner, 2017-01-09 22:49:26]

The CQ bit was checked by eakuefner@chromium.org

[commit-bot: I haz the power, 2017-01-09 22:49:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-09 22:53:08]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484002166893840,
"parent_rev": "886ff596e4884a69b7977a6929165e239955ae5b", "commit_rev":
"3117a26ce459be98f34a94cf0437ad5e300c21e3"}

[commit-bot: I haz the power, 2017-01-09 22:53:11]

Description was changed from

==========
[StyleGuide] Allow const in JavaScript code

This CL allows use of the const keyword in JavaScript code, after offline
discussion with benjhayden@ and charliea@.

BUG=catapult:#2833
==========

to

==========
[StyleGuide] Allow const in JavaScript code

This CL allows use of the const keyword in JavaScript code, after offline
discussion with benjhayden@ and charliea@.

BUG=catapult:#2833

Review-Url: https://codereview.chromium.org/2466263003
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-01-09 22:53:12]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...