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

Base indent for multiple line breaks; npm #126

Closed
wants to merge 4 commits into from
Closed

Conversation

brettz9
Copy link
Contributor

@brettz9 brettz9 commented Oct 31, 2019

  • Fix: Apply base indent to multiple line breaks; amend test to check
  • Testing: Get plugins tests to work with eslint 6
  • Teting: Expect an extra line break since remark-parse@7 started preserving them (see [CommonMark] Leading/trailing newlines are stripped in the value of fenced codeblock remarkjs/remark#351 )
  • Linting: Remove call to lint absent Makefile.js
  • Linting: Add ignore file and explicitly exclude undesired
  • npm: Update object-assign, unified, remark-parse deps.
  • npm: Update eslint-release, chai, mocha, eslint devDeps
  • npm: Change from deprecated istanbul to nyc
  • npm: Add ignore file

Note that the only dep./devDep. that had an upgrade but which I didn't upgrade was eslint-plugin-node, as eslint-config-eslint is still tied to an older version thereof.

Btw, the reason I changed to explicitly ignoring more files was so that the likes of my IDE linter wouldn't try to lint files which you hadn't excluded, even though your npm script wasn't targeted at them previously. I also figured it safer to lint all by default and just exclude the ones you don't want linted.

Update object-assign, unified, remark-parse for deps and eslint-release, chai, mocha, eslint for devDeps.
In process of updating:

1. Get plugins tests to work with eslint 6
2. In whitespace processor test, add an extra line break for the expected since `remark-parse@7` started preserving them (see remarkjs/remark#351 )
In the process, add to the ignore file and explicitly exclude undesired
@brettz9
Copy link
Contributor Author

brettz9 commented Oct 31, 2019

Btw, I didn't update travis.yml to handle the eslint@6 upgrade (though I can if you wish), since a number of projects (such as eslint itself) have been dropping Node 6 support with it reaching end-of-life, and I didn't know if you might wish to also (in which case, it would just be a matter of dropping that Node line from travis.yml).

And I'm not sure why the commit message is still failing despite my attempting to follow the eslint guidelines.

@btmills
Copy link
Member

btmills commented Oct 31, 2019

Hi @brettz9, thanks for taking the initiative to do all of this! As-is, this pull request does too many different things, so let's split it up into a couple:

  1. I want to get Fixer to apply indent for multiple line breaks #127 fixed right away, so let's start with a pull request with just 538c442.
  2. A pull request with f924c99 would also be good - I see why the .eslintignore approach makes it easier to work on this with editor integrations. If you want to include it in that same PR, I'd also welcome a second commit that stops ignoring tests/lib/processor.js and runs eslint --fix and fixes any other issues so that it passes. I'm not sure why that file was ever omitted in the first place.

I'm less convinced of need for the .npmignore file and dependency upgrades, but if you'd like to either open issues or pull requests for those I'm happy to discuss them there. Short version here:

I'll keep this open for now to track the changes to fix #127 and modify .eslintignore, and then this can be closed once those pull requests exist.

@brettz9
Copy link
Contributor Author

brettz9 commented Oct 31, 2019

I've filed #128 and #129 (though not sure why that commit message is failing unless it needs an issue despite the docs suggesting it wouldn't). Re: npmignore, I always forget to check for files, yeah, I see not needed.

@brettz9 brettz9 closed this Oct 31, 2019
brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Oct 31, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps. and devDeps; istanbul -> nyc

Update object-assign, unified, remark-parse for deps and eslint-release, chai, mocha, eslint for devDeps.
In process of updating:

1. Get plugins tests to work with eslint 6
2. In whitespace processor test, add an extra line break for the expected since `remark-parse@7` started preserving them (see remarkjs/remark#351 )
@brettz9 brettz9 deleted the npm branch November 1, 2019 00:06
@brettz9
Copy link
Contributor Author

brettz9 commented Nov 1, 2019

Btw, I see the processor API is already ready with eslint/eslint#11552 and the docs updated: https://eslint.org/docs/developer-guide/working-with-plugins .

FWIW, in applying the new processor API, you may still find my work updating the deps/devDeps helpful as it took me some time to figure out, e.g., my not finding anywhere that plugins was now needed with CLIEngine; I have the branch at https://github.com/brettz9/eslint-plugin-markdown/tree/deps-devDeps .

brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Nov 1, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps. and devDeps; istanbul -> nyc

Update object-assign, unified, remark-parse for deps and eslint-release, chai, mocha, eslint for devDeps.
In process of updating:

1. Get plugins tests to work with eslint 6
2. In whitespace processor test, add an extra line break for the expected since `remark-parse@7` started preserving them (see remarkjs/remark#351 )
brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Nov 1, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps. and devDeps; istanbul -> nyc

Update object-assign, unified, remark-parse for deps and eslint-release, chai, mocha, eslint for devDeps.
In process of updating:

1. Get plugins tests to work with eslint 6
2. In whitespace processor test, add an extra line break for the expected since `remark-parse@7` started preserving them (see remarkjs/remark#351 )
@btmills
Copy link
Member

btmills commented Nov 1, 2019

you may still find my work updating the deps/devDeps helpful as it took me some time to figure out

Thanks so much for the reference! I appreciate having that available when I get to it.

@btmills
Copy link
Member

btmills commented Nov 1, 2019

If you're willing to pull just the devDependencies changes from 9a65621 into another Chore: ... PR I'd be happy to merge that too! I should've thought about that distinction in #126 (comment) - it's the remark and unified upgrades that I want to wait to do.

brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Nov 1, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps.

Update object-assign, unified, remark-parse for deps. In process of updating,
for whitespace processor test, add an extra line break
for the expected since `remark-parse@7` started preserving them
(see remarkjs/remark#351 )
brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Nov 1, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps.

Update object-assign, unified, remark-parse for deps. In process of updating,
for whitespace processor test, add an extra line break
for the expected since `remark-parse@7` started preserving them
(see remarkjs/remark#351 )
brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Nov 1, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps.

Update object-assign, unified, remark-parse for deps. In process of updating,
for whitespace processor test, add an extra line break
for the expected since `remark-parse@7` started preserving them
(see remarkjs/remark#351 )
brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Nov 1, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps.

Update object-assign, unified, remark-parse for deps. In process of updating,
for whitespace processor test, add an extra line break
for the expected since `remark-parse@7` started preserving them
(see remarkjs/remark#351 )
brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Nov 1, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps.

Update unified, remark-parse for deps. In process of updating,
for whitespace processor test, add an extra line break
for the expected since `remark-parse@7` started preserving them
(see remarkjs/remark#351 )
brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Nov 2, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps.

Update unified, remark-parse for deps. In process of updating,
for whitespace processor test, add an extra line break
for the expected since `remark-parse@7` started preserving them
(see remarkjs/remark#351 )
brettz9 added a commit to brettz9/eslint-plugin-markdown that referenced this pull request Nov 3, 2019
…ps://github.com/eslint/rfcs/tree/master/designs/2018-processors-improvements / eslint/eslint#11552

Upgrade: Update deps.

Update unified, remark-parse for deps. In process of updating,
for whitespace processor test, add an extra line break
for the expected since `remark-parse@7` started preserving them
(see remarkjs/remark#351 )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants