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

Include more reservedPaths. Fixes #1 #2

Merged
merged 6 commits into from
Jun 24, 2017
Merged

Include more reservedPaths. Fixes #1 #2

merged 6 commits into from
Jun 24, 2017

Conversation

Mottie
Copy link
Contributor

@Mottie Mottie commented Jun 24, 2017

I ran npm test and found these messages which I did not tackle in this PR:

index.js
  ‼   66:1   Function shortenURL has a complexity of 22.  complexity
  ‼  146:29  Do not nest ternary expressions              no-nested-ternary

@Mottie
Copy link
Contributor Author

Mottie commented Jun 24, 2017

Oh LOL

index.js Outdated
'organizations',
'notifications',
];
const reservedPaths = require('github-reserved-names').all;
Copy link
Member

Choose a reason for hiding this comment

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

require should be at the top to make it easier to switch to import in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

@fregante
Copy link
Member

Looking good! Do you mind requiring the JSON directly? It saves a require trip

@Mottie
Copy link
Contributor Author

Mottie commented Jun 24, 2017

Do you mind requiring the JSON directly?

Do you mean like this?

const reservedPaths = require("./node_modules/github-reserved-names/reserved-names.json");

@fregante
Copy link
Member

const reservedPaths = require("github-reserved-names/reserved-names.json");

@Mottie
Copy link
Contributor Author

Mottie commented Jun 24, 2017

LOL cool... so many ways to do the same thing ;)

@fregante fregante merged commit 40b308a into refined-github:master Jun 24, 2017
@Mottie
Copy link
Contributor Author

Mottie commented Jun 24, 2017

LOL just as you merge, I found another reserved name... I'll publish it in a minute.

@fregante
Copy link
Member

If it matches ^1.0.3 it'll be in automatically :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants