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

Fix unicode checks for fallbacks with test #125

Merged
merged 3 commits into from
Apr 25, 2019

Conversation

moranje
Copy link
Contributor

@moranje moranje commented Apr 22, 2019

No description provided.

There is no way to provide a unicode fallback pattern currently.

Error: If one rule is /u then all must be
@moranje
Copy link
Contributor Author

moranje commented Apr 22, 2019

Test passes. Also I noticed two things when I cloned project from scratch:

  • The package in it's current form has 268 security vulnerabilities npm audit fix --force (which only seems to update jest) leaves two.
  • The project may benefit from prettier (I have a prettier running in the background on every project by default and it tries to eliminate inconsistencies in the files. I've tried to create a .prettierrc that would have the least amount of changes which seems to be:
{
  "printWidth": 80,
  "tabWidth": 2,
  "useTabs": false,
  "semi": false,
  "singleQuote": true,
  "trailingComma": "es5",
  "bracketSpacing": false,
  "arrowParens": "avoid"
}

I'd be willing to create pull-requests for these changes if you would welcome these.

@tjvr
Copy link
Collaborator

tjvr commented Apr 23, 2019

Yeah, NPM always seems to have audit failures because I don't upgrade dependencies very often. Which is silly, since Moo itself has no dependencies 🙂 A PR to upgrade the deps would be most welcome.

I've thought about adding prettier before, and possibly have a branch somewhere. It might be worth it (I really like go fmt, which it appears to emulate).

I'll try and review this PR later, thanks 😊

Sent with GitHawk

Copy link
Collaborator

@tjvr tjvr left a comment

Choose a reason for hiding this comment

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

Thanks for this! 🎉

@tjvr tjvr merged commit cf9b14f into no-context:master Apr 25, 2019
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