gabiseabra / google-fonts-webpack-plugin

129 stars 44 forks source link

Check on existing callback #22

Open navelpluisje opened 6 years ago

navelpluisje commented 6 years ago

Removed callback from html-webpack-plugin-before-html-generation, because it is not returning a callback (anymore). fixes #19

tatobari commented 6 years ago

Should the callback be removed or, to retain backward compatibility, should the callback function be called with the following validation?

if(typeof cb == 'function'){
  cb(null, data);
}

I'm definitely out of my place here cause I'm no programmer, but looks safer to me.

navelpluisje commented 6 years ago

@tatobari Good point. I did some tests before this change and did not get any callback function returned, that's why I removed it. But for backward compatibility I will change it.

And now let's hope it will be merged

tatobari commented 6 years ago

@navelpluisje Awesome man! Let's hope @gabiseabra has time to check your PR.

tatobari commented 6 years ago

@navelpluisje Hate to say this, but line 116 says cd instead of cb. Little code TYPO, I guess.

navelpluisje commented 6 years ago

@tatobari Changed it. At least it couldn't break anything ;-)

tatobari commented 6 years ago

@navelpluisje Job well done, sir.

ymhr commented 6 years ago

Is there any chance of this being accepted soon?

unel commented 6 years ago

is here a some life exists?..

antony commented 6 years ago

I'm assuming this project has been abandoned, so I've forked, merged this PR and published the module as @beyonk/google-fonts-webpack-plugin - also on github.

Please test and let me know your findings!

subins2000 commented 4 years ago

Thank you for maintaining this @antony . If you intent on maintaining it, can you please make a separate repo (instead of fork) so we can file issues and work on it there ?

antony commented 4 years ago

A fork is a separate repository. You can file issues there :)

subins2000 commented 4 years ago

@antony I don't see an Issues tab there tho ?

Screenshot_20200707_140724

antony commented 4 years ago

Done.

Btw I don't plan on active maintenance, we use rollup now. But if people open PRs I will review, merge and release.

subins2000 commented 4 years ago

@antony Just sent a PR there :)