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

messages/javascript: replace tslint with eslint #837

Merged
merged 14 commits into from
Jan 14, 2020
Merged

Conversation

davidjgoss
Copy link
Contributor

Summary

This replaces TSLint with ESLint and appropriate tooling and config.

Details

Using equivalents as much as possible, e.g.

  • tslint:latest -> eslint:recommended
  • tslint-config-prettier -> eslint-config-prettier

and minimal actual code change.

Motivation and Context

TSLint is deprecated and its maintainers now recommend ESLint.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • The change has been ported to Java.
  • The change has been ported to Ruby.
  • The change has been ported to JavaScript.
  • The change has been ported to Go.
  • The change has been ported to .NET.
  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

- switch off a couple of eslint rules it looks like we don't want
- use eslint-plugin-node for equivalent of "no-implicit-dependencies"
- slight refactor to avoid no-constant-condition
@@ -5,7 +5,6 @@ coverage/
node_modules/
yarn.lock
npm-debug.log
package-lock.json
Copy link
Contributor Author

@davidjgoss davidjgoss Dec 20, 2019

Choose a reason for hiding this comment

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

Not sure if this was intentional originally, but meant volatile builds were possible unless I missed something.

@@ -15,13 +15,14 @@ export default class BinaryToMessageStream<T> extends Transform {

public _transform(chunk: any, encoding: string, callback: TransformCallback) {
this.buffer = Buffer.concat([this.buffer, chunk])

while (true) {
let finished = false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only actual code change - seemed worthwhile to avoid ignoring the no-constant-condition rule which would otherwise be useful. This is covered by an existing test.

@davidjgoss
Copy link
Contributor Author

Looking for feedback at this point before doing anything with other TypeScript packages in this repo e.g. expressions

@aslakhellesoy aslakhellesoy added language: javascript 🔧 build Related to build / release process labels Dec 24, 2019
@aslakhellesoy
Copy link
Contributor

I'm close to finishing this one. Because we use shared linting config across packages this turned out to be quite big, but important to get done I think.

@aslakhellesoy
Copy link
Contributor

@vincent-psarga I also removed the package-lock.json and added them to .gitignore in this PR. This was needed in order to avoid these kinds of build errors that also happened on Bastien's machine yesterday.

@aslakhellesoy aslakhellesoy merged commit 8df5e26 into master Jan 14, 2020
@aslakhellesoy aslakhellesoy deleted the eslint branch January 14, 2020 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language: javascript 🔧 build Related to build / release process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants