Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-25176] Fix node 4 compatibility #9322

Merged
merged 1 commit into from Aug 16, 2017

Conversation

ewanharris
Copy link
Collaborator

@ewanharris ewanharris commented Aug 16, 2017

JIRA: https://jira.appcelerator.org/browse/TIMOB-25176

This PR add node 4 compatibility back, I only added 'use strict' to the files we distribute, so files in apidoc/, build/ and Gruntfile.js will still break on node 4.

If we want that it's just a matter of me running the relevant jscodemod again so no biggy, just let me know

@ewanharris ewanharris changed the title Fix node 4 compatibility [TIMOB-25176] Fix node 4 compatibility Aug 16, 2017
@@ -5,8 +5,9 @@
* See the LICENSE file for more information.
*/

const
appc = require('node-appc'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgtcoolguy Wondering if that format was expected with eslint and may be in other files as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this is my super old coding style. It's bad. Nowadays I specify const or let per variable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 On each variable declaration being separate, as always there's a rule for that but not sure whether all teams would buy in to having that in the config. There's also a rule for strict mode which imo if our min ver will be node 4 should be added to the config (again doubt all times would buy in though)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think we can still do per-project overrides, or additional rules on top of the common config. I'd agree that the "strict": "safe" rule is probably a good one to turn on while Node 4 is our min supported Node.

Personally, I like having multiple const/var/let` declarations combined.

@sgtcoolguy sgtcoolguy merged commit 9db40e7 into tidev:master Aug 16, 2017
@sgtcoolguy
Copy link
Contributor

I merged this, added the eslint rule to our .eslintrc - as well as tweaked the parserOptions to specify sourceType: script and ecmaVersion: 2015, then added 'use strict' to the remaining files that eslint flagged: 0bef6bf

@ewanharris ewanharris deleted the use_strict branch August 31, 2021 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants