Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

No cond assign eslint implementation #116

Merged
merged 5 commits into from
Mar 3, 2020
Merged

Conversation

macovedj
Copy link
Contributor

@macovedj macovedj commented Mar 3, 2020

Going through #94. eslint has two options for this rule, one that allows exception via parens, and the other which would need ignoring to escape. I implemented the latter, because I don't think people should be encouraged to use paren, but I could implement it with parens if solid reasoning were provided.

@macovedj
Copy link
Contributor Author

macovedj commented Mar 3, 2020

eslint has two options for this rule, one that allows exception via parens, and the other which would need ignoring to escape. I implemented the latter, because I don't think people should be encouraged to use paren, but I could implement it with parens if solid reasoning were provided.

Lol. The project currently violates this lint rule if we don't allow parens right here. It feels to me like there's a better way to do this, which I hope to have time to figure out tomorrow, but also I now see why parens would be desired. What do you think @sebmck?

@sebmck
Copy link
Contributor

sebmck commented Mar 3, 2020

Heh, I believe that's code left over from babel-generator. You should be able to replace that with:

while (true) {
  const item = this._queue.pop();
  if (item === undefined) {
    break;
  } else {
    this._append(...item);
  }
}

Makes it a lot clearer what the code is doing.

Feel free to do that in this PR!

@sebmck
Copy link
Contributor

sebmck commented Mar 3, 2020

The rest of the PR looks good to me!

@sebmck sebmck mentioned this pull request Mar 3, 2020
27 tasks
@macovedj
Copy link
Contributor Author

macovedj commented Mar 3, 2020

Heh, I believe that's code left over from babel-generator. You should be able to replace that with:

while (true) {
  const item = this._queue.pop();
  if (item === undefined) {
    break;
  } else {
    this._append(...item);
  }
}

Makes it a lot clearer what the code is doing.

Feel free to do that in this PR!

Done!

@sebmck sebmck merged commit a7efa62 into rome:master Mar 3, 2020
@sebmck
Copy link
Contributor

sebmck commented Mar 3, 2020

Looks good, thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants