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: false negative of max-len (fixes #6564) #6565

Merged
merged 1 commit into from Jul 3, 2016

Conversation

not-an-aardvark
Copy link
Member

Fixes #6564.

This corrects a bug in the isFullLineComment helper function, which was incorrectly identifying non-commented lines of code as full-line comments if they were preceded by a commented line of the same length.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @vitorbal, @kaicataldo and @bgw to be potential reviewers

@eslintbot
Copy link

LGTM

@jquerybot
Copy link

Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA).

📝 Please visit http://contribute.jquery.org/CLA/ to sign.

After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know.


If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check.

@eslintbot
Copy link

LGTM

@not-an-aardvark
Copy link
Member Author

I think I've signed the CLA correctly; I'm not sure why jquerybot still has the CLA: Error label.

@platinumazure
Copy link
Member

platinumazure commented Jun 30, 2016

The CLA check details says something about name "not-an-aardvark" needing manual verification. I'm not sure what that means.

To be honest, this PR won't be merged until the issue is deliberated upon and accepted (hopefully it will take very little time to deliberate), so I think we might as well let that take its course. Whichever ESLint team member accepts the issue will also see the PR and become aware of the CLA issue. Unfortunately I don't know how to actually remedy CLA issues, I just read the check status pages. 😄

@platinumazure
Copy link
Member

platinumazure commented Jun 30, 2016

@not-an-aardvark I just took a spin through the repo which I think powers the license check bot, jquery/jquery-license. Found this validation file, which seems to have a heuristic where a username is split by spaces and if there's only one name, then that message is emitted.

I don't know if "manual verification" means someone on the jQuery team can let this through. But what you can definitely do for sure is sign the CLA under your own name (unless you have one name, I guess) and then configure your local Git to sign commits with that name.

Briefly:

  1. Sign CLA with your real name and the e-mail address you used
  2. In your local git repo, run git config user.name Real Name (or git config --global user.name Real Name to apply to all your local repos)
  3. Run git commit --amend --no-edit to amend your previous commit, leaving the commit message and changes intact but using your new configured username
  4. Run git push origin your-branch -f (where "origin" is the remote name for your fork of ESLint) to force-push the commit onto your branch
  5. Hopefully the jQuery bot will set this to CLA: Valid at that point

Hope this helps.

@eslintbot
Copy link

LGTM

@ilyavolodin
Copy link
Member

@kborchers Would you mind helping us with CLA again?

@eslintbot
Copy link

LGTM

@ilyavolodin ilyavolodin merged commit 25fc7b7 into eslint:master Jul 3, 2016
@not-an-aardvark not-an-aardvark deleted the issue6564 branch July 3, 2016 05:34
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants