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

Fix: ignore lines with empty elements (fixes #12756) #14837

Merged
merged 3 commits into from Jul 30, 2021

Conversation

boutahlilsoufiane
Copy link
Contributor

@boutahlilsoufiane boutahlilsoufiane commented Jul 25, 2021

Prerequisites checklist

  • I have read the contributing guidelines.
  • The team has reached consensus on the changes proposed in this pull request. If not, I understand that the evaluation process will begin with this pull request and won't be merged until the team has reached consensus.

What is the purpose of this pull request? (put an "X" next to an item)

[X] Bug fix

What changes did you make? (Give an overview)

Fix the issue here: #12756

This PR fixes a bug in the rule comma-style, it ignores lines that are only made of empty elements.
This is an example:

const [
    ,
    a,
    b,
] = arr;

Is there anything you'd like reviewers to focus on?

No.

@eslint-github-bot eslint-github-bot bot added the triage An ESLint team member will look at this issue soon label Jul 25, 2021
@eslint-github-bot
Copy link

Hi @boutahlilsoufiane!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

3 similar comments
@eslint-github-bot
Copy link

Hi @boutahlilsoufiane!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

@eslint-github-bot
Copy link

Hi @boutahlilsoufiane!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

@eslint-github-bot
Copy link

Hi @boutahlilsoufiane!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

@boutahlilsoufiane boutahlilsoufiane changed the title Fix: Ignore any lines that are only made of empty elements (fixes #12… Fix: Ignore lines with empty elements (fixes #12… Jul 25, 2021
@eslint-github-bot
Copy link

Hi @boutahlilsoufiane!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

@boutahlilsoufiane boutahlilsoufiane changed the title Fix: Ignore lines with empty elements (fixes #12… Fix: ignore lines with empty elements (fixes #12… Jul 25, 2021
@eslint-github-bot
Copy link

Hi @boutahlilsoufiane!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

@eslint-github-bot
Copy link

Hi @boutahlilsoufiane!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The issue reference must be formatted as follows:

    If the pull request addresses an issue, then the issue number should be mentioned at the end. If the commit doesn't completely fix the issue, then use (refs #1234) instead of (fixes #1234).

    Here are some good commit message summary examples:

    Build: Update Travis to only test Node 0.10 (refs #734)
    Fix: Semi rule incorrectly flagging extra semicolon (fixes #840)
    Upgrade: Esprima to 1.2, switch to using comment attachment (fixes #730)
    

Read more about contributing to ESLint here

@boutahlilsoufiane boutahlilsoufiane changed the title Fix: ignore lines with empty elements (fixes #12… Fix: ignore lines with empty elements (fixes eslint#12756) Jul 25, 2021
@eslint-github-bot
Copy link

Hi @boutahlilsoufiane!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The issue reference must be formatted as follows:

    If the pull request addresses an issue, then the issue number should be mentioned at the end. If the commit doesn't completely fix the issue, then use (refs #1234) instead of (fixes #1234).

    Here are some good commit message summary examples:

    Build: Update Travis to only test Node 0.10 (refs #734)
    Fix: Semi rule incorrectly flagging extra semicolon (fixes #840)
    Upgrade: Esprima to 1.2, switch to using comment attachment (fixes #730)
    

Read more about contributing to ESLint here

@nzakas nzakas changed the title Fix: ignore lines with empty elements (fixes eslint#12756) Fix: ignore lines with empty elements (fixes #12756) Jul 29, 2021
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up. The change looks good, just a note on the tests.

tests/lib/rules/comma-style.js Outdated Show resolved Hide resolved
@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Jul 29, 2021
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. I’d just like another set of eyes before merging.

@btmills btmills merged commit d7dc07a into eslint:master Jul 30, 2021
@boutahlilsoufiane boutahlilsoufiane deleted the issue12756 branch July 31, 2021 19:12
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jan 27, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants