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

Tweaks #74

Merged
merged 15 commits into from
Oct 9, 2018
Merged

Tweaks #74

merged 15 commits into from
Oct 9, 2018

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Oct 9, 2018

@tschaub: sorry for not splitting this, I could try splitting it if you want later.

Now, this fixes tests on Windows with autocrlf enabled, updates all the stuff and fixes a DeprecationWarning with node >= 8 when running the tests.

The only thing left is to replace grunt-cafe with something else.

Do let me know if you want me to change something and I can rebase this as needed :)

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 9, 2018

If you have an ES6+ ESLint config, we should switch to that to ensure everything is compliant in the future. Not a stopper, but would be nice to have.

@tschaub tschaub merged commit 2f63665 into tschaub:master Oct 9, 2018
@tschaub
Copy link
Owner

tschaub commented Oct 9, 2018

Thanks a lot @XhmikosR! I'll give you rights to maintain this package if you are interested. I'd be glad to pass off responsibility for it (as I'm not an active grunt user).

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 9, 2018

Haha, me neither :)

I just still use it in a few projects so I thought we'd tweak this.

You can add me, no problem, I just can't make any promises I'll keep contributing for a long time :)

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 9, 2018

@tschaub: Here's the things I'd like to fix soon-ish for the next release

  1. I think we should fix Replace the deprecated fs.exists() #75 and cut a new release, what do you think? Can you take care of that?
  2. Also, how about an ES6 ESlint config? Do you have one we could use?
  3. tasks\gh-pages.js:85:5 The body of a for-in should be wrapped in an if statement to filter unwanted properties from the prototype.
  4. tasks\gh-pages.js:122:5 Promise.then() requires 1 or 2 arguments, but received 3

@tschaub
Copy link
Owner

tschaub commented Oct 9, 2018

I think working on #76 would provide the most benefit (remove the most code). But feel free to prioritize as you see fit – I won't have any real time to work on this.

Regarding the ESLint config, the current config is set up for ES2018.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 9, 2018

Hmm, it seems the config isn't enforcing all ES6 features like no var, prefer arrow, template literals etc. Not a big deal, just wanted to make this consistent in the future.

And yeah, I agree, since you have that package, that should be the first priority and should fix of the issues I mentioned. If not we can sort them upstream and every package will benefit.

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.

2 participants