Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(967)

Issue 2621643003: [StyleGuide] Allow let in JavaScript code (Closed)

Created:
3 years, 11 months ago by eakuefner
Modified:
3 years, 10 months ago
CC:
catapult-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

[StyleGuide] Allow let in JavaScript code In https://codereview.chromium.org/2466263003 we realized that we should think about our strategy for allowing `let` in Catapult. charliea@ proposed three options for moving to allow it: * Allow `let` but don't recommend it * Recommend it for new code, but not for old code * Recommend it for new code and require fixes to old code as it is modified. On the basis that block scoping is a win for developer productivity, it makes sense to recommend `let` in at least some cases, which throws out the first option. Given that requiring fixes to old code will almost certainly annoy people, and the promise of --fix one day working with processors, that seems to leave the second option as the best possible one. BUG=catapult:#2833 Review-Url: https://codereview.chromium.org/2621643003 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/98ef250d15b5d10ea61de27268f830cb07ca48a2

Patch Set 1 #

Patch Set 2 : clarify required for new code #

Patch Set 3 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M docs/style-guide.md View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 16 (7 generated)
eakuefner
PTAL.
3 years, 11 months ago (2017-01-09 22:36:59 UTC) #2
charliea (OOO until 10-5)
lgtm I'm assuming we'll allow people to migrate old code to use it, but won't ...
3 years, 11 months ago (2017-01-12 18:29:27 UTC) #3
benjhayden
lgtm
3 years, 11 months ago (2017-01-12 18:58:25 UTC) #4
eakuefner
On 2017/01/12 at 18:29:27, charliea wrote: > lgtm > > I'm assuming we'll allow people ...
3 years, 11 months ago (2017-01-12 19:04:00 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2621643003/20001
3 years, 11 months ago (2017-01-12 19:04:10 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: Catapult Linux Tryserver on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Linux%20Tryserver/builds/6210) Catapult Presubmit ...
3 years, 11 months ago (2017-01-12 19:05:20 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2621643003/40001
3 years, 10 months ago (2017-01-27 18:03:00 UTC) #12
commit-bot: I haz the power
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/98ef250d15b5d10ea61de27268f830cb07ca48a2
3 years, 10 months ago (2017-01-27 18:24:06 UTC) #15
lpy
3 years, 10 months ago (2017-01-27 18:24:56 UTC) #16
Message was sent while issue was closed.
Awesome! Thank you!

Powered by Google App Engine
This is Rietveld 408576698