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

Replace deprecated gulp-util #5

Merged
merged 17 commits into from Mar 7, 2018

Conversation

ScorpioCoding
Copy link
Contributor

Hi there,

Created a hotfix for the gulp-util deprecation issue
Installed dependecies

  • plugin-error as replacement for gutil.PluginError
  • fancy-log to replace gutil.log
  • ansi-colors to replace gutil.colors
  • vinyl to replace gutil.File

files modified are:
.gitignore
--- added package-lock.json that occures when using VS code and SourceTree Git manager
package.json
--- added dependencies
index.js
--- mod code

I hope that my first try is a succes?
my firdst pull-request....lol

kind regards
scorpiocoding - kribo

@meeroslav
Copy link
Owner

@ScorpioCoding, thank's for help. Please fix the failing unit tests in order to accept this pull request.

@demurgos
Copy link

demurgos commented Jan 3, 2018

@meeroslav
Hi there, I'm tracking the gulp-util migration in this issue, check it for more information. I can't send PRs to everyone but I subscribed to this PR and will review it once CI passes if you want.

@ScorpioCoding
I posted some comments regarding your PRs in your run-sequence PR. This PR here is not a duplicate so if you fix the issues it should be fine (if @meeroslav agrees to merge).

@ScorpioCoding
Copy link
Contributor Author

@meeroslav
would luv to fix the unit testing but it keeps giving me a nodejs error - set env variable.
Zo where do I set the env variable?
Or what simple thing have I forgotten?
Can u advice plz?

@ScorpioCoding
Copy link
Contributor Author

@demurgos
I read your comments and I have taken them to hart and modded a few open PR's
Thnx for the sugestions..

@meeroslav
Copy link
Owner

meeroslav commented Jan 4, 2018

@denOldTimer or @ScorioCoding, you can check travis for more details https://travis-ci.org/meeroslav/gulp-inject-partials/jobs/324991485.

You cannot simply replace gutil.log with fancyLog in the beforeEach as you then simply create new pointer which is not linked to anything. Since fancyLog calls console.log under the hood, you can override console.log instead of old gutil.log.

@meeroslav meeroslav changed the title Scorpiocoding Replace deprecated gulp-util Jan 4, 2018
@ScorpioCoding
Copy link
Contributor Author

@meeroslav
Keep getting an AssertionError on line 127 of index.spec.js

@@ -12,15 +18,15 @@ describe('gulp-inject-partials', function(){
var logOutput = [];

beforeEach(function () {
log = gutil.log;
log = fancyLog.log;
Copy link

@demurgos demurgos Jan 4, 2018

Choose a reason for hiding this comment

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

I'd recommend to use fancyLog.info (everywhere in this package) because of this kind of test relying on Node's cache to patch shared dependencies. (fancyLog.log does not exist)

I personally consider it a bit brittle so I exposed the logger on the main module in another PR. Finally, a solution would be to use proxy-require to stub fancy-log.

@meeroslav
Copy link
Owner

I will fix this on the master, since it's simpler that way. Sorry for waiting so long.

@meeroslav meeroslav merged commit 0b86150 into meeroslav:master Mar 7, 2018
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

4 participants