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.

Sign up for GitHub

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

Jump to bottom

feat: support globals #81

Merged
merged 1 commit into from
Sep 5, 2017
Merged

feat: support globals #81

merged 1 commit into from
Sep 5, 2017

Conversation

AndersDJohnson
Copy link
Contributor

Fixes #79

@jsf-clabot
Copy link

jsf-clabot commented Jul 7, 2017 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

vsemozhetbyt
vsemozhetbyt reviewed Jul 7, 2017
View reviewed changes
lib/processor.js Outdated
@@ -58,7 +58,7 @@ function getComment(html) {

html = html.slice(commentStart.length, -commentEnd.length);

if (html.trim().slice(0, prefix.length) !== prefix) {
if (!html.trim().match(regex)) {
Copy link

@vsemozhetbyt vsemozhetbyt Jul 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe !regex.test(html.trim()) would be more efficient in a boolean context (returns boolean value, not object)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vsemozhetbyt Good call, pushed update.

@AndersDJohnson
Copy link
Contributor Author

@vsemozhetbyt I made the requested change, does this look good now?

@vsemozhetbyt
Copy link

vsemozhetbyt commented Jul 16, 2017 •
edited
Loading

@AndersDJohnson Yes, for me, but I am not a collaborator in this repository and have not any approval rights)

@vsemozhetbyt
Copy link

If I get it right, this repository is updated seldom, so be not discouraged with a long review delay.

@btmills
Copy link
Member

btmills commented Sep 5, 2017

this repository is updated seldom

Sorry for the delay, @AndersDJohnson! Yeah, I work on this in my spare time in between other projects, so it can take awhile.

btmills
btmills approved these changes Sep 5, 2017
View reviewed changes
Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @AndersDJohnson!

@btmills btmills merged commit 7c19f8b into eslint:master Sep 5, 2017
btmills added a commit that referenced this pull request Sep 5, 2017
@btmills
Chore: Replace global comment integration test with unit test (refs #81)
ce49292
btmills added a commit that referenced this pull request Sep 11, 2017
@btmills
Chore: Replace global comment integration test with unit test (refs #81…
47ad3f9 
…) (#85)
@AndersDJohnson AndersDJohnson deleted the global branch December 31, 2017 13:11
@AndersDJohnson
Copy link
Contributor Author

AndersDJohnson commented Dec 31, 2017 •
edited
Loading

@btmills Will you be releasing a new version, even a beta one, to include this and other changes? Thanks! Will #86 include?

@btmills
Copy link
Member

btmills commented Apr 8, 2018

@AndersDJohnson sorry for the delay on this. 1.0.0-beta.8 is published with this change! npm install --save-dev eslint-plugin-markdown@next.

@github-actions github-actions bot mentioned this pull request Jul 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsemozhetbyt vsemozhetbyt vsemozhetbyt left review comments

@btmills btmills btmills approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AndersDJohnson @jsf-clabot @vsemozhetbyt @btmills