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

Exporting the depcheck ‘special’ property so it can be used in the op… #9

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

edj-boston
Copy link

I added a special property to the gulp plugin that is a duplicate of the depcheck dependency inside the plugin itself. It allows you to bring in special parsers and pass them as options, like so:

var gulpDepcheck = require('gulp-depcheck');

gulp.task('package', gulpDepcheck({
    ignoreMatches : [ 'bootstrap',  'jquery' ],
    specials : [
        gulpDepcheck.special.babel,
        gulpDepcheck.special['gulp-load-plugins']
    ]
}));

Notes:
1.) If someone passes their own version of depcheck as an option, it will not be used by the special property. But they can pass their own specials in like so:

var depcheck = require('depcheck');
var gulpDepcheck = require('gulp-depcheck');

gulp.task('package', gulpDepcheck({
    depcheck : depcheck,
    ignoreMatches : [ 'bootstrap',  'jquery' ],
    specials : [
        depcheck.special.babel,
        depcheck.special['gulp-load-plugins']
    ]
}));

2.) I could not get your code to build on my machine:

[16:14:38] Starting '_license'...
[
  {
    package: 'has',
    version: '1.0.1',
    license: 'UNKNOWN'
  }
]
[16:14:39] '_license' errored after 112 ms

But it did lint at least.

Copy link
Contributor

@goloroden goloroden left a comment

Choose a reason for hiding this comment

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

Thank you so much for sending a PR! It looks great, there are only a few really minor issues that I would like to ask you to address (see comments).

Apart from that, if you want to, would you mind adding yourself as a contributor to the package.json file? Once these things are done, I am happily going to merge this 😊

README.md Outdated

```javascript
gulp.task('depcheck', depcheck({
specials : [
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please change this to indentation with two spaces instead of four?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

README.md Outdated
const depcheck = require('gulp-depcheck');
const myCustomDepcheck = require('depcheck');

gulp.task('depcheck', depcheck({
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please change this to indentation with two spaces instead of four?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

README.md Outdated
Note that if you use a custom version of depcheck you must pass any special parsers explicitly.

```javascript
const depcheck = require('gulp-depcheck');
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please remove the second const, indent the line with six spaces, and add a comma to the end of the first line?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@@ -1,6 +1,7 @@
'use strict';

const _ = require('lodash'),
dc = require('depcheck'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please rename dc to depcheck, and also change the subsequent ones?

Copy link
Author

Choose a reason for hiding this comment

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

There will be a variable naming conflict between the global depcheck and the const depcheck within the gulpDepcheck scope. Do you have a different preferred name, or another workaround?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, okay, I see… hmmm. What do you think about this one:

  • The global one is depcheck, as it represents the module.
  • The local one could be called depcheckInstance, as this now is the specific one you are going to deal with.

What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense! Fixed.

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

Successfully merging this pull request may close these issues.

None yet

2 participants