Closed Garbee closed 7 years ago
Ok, I'll get it into its own function and address the other things tomorrow.
Thank you.
So, dumb me forgot for a moment JS functions just go up the scope chain when you're calling a variable and it doesn't exist in the current scope. So the fat arrow wasn't even necessary, yay.
Just moved the check into a function, that takes the current valid state and the scope to check as parameters. At the end, returning the valid state for the main process to continue as normal.
The add-contributor method also works just fine. I just didn't press spacebar
to select something, I assumed enter
would do that if I only was on one with the cursor.
So, the declarations are changed and the function is moved to the bottom with this update. However, the inner function to validate a single scope is still contained within the function instead of being on its own. Because, it is relying on the variables within the scope it is defined so it can be as simple as possible.
If it were to get moved out, we'd need to pass in the allowedScopes
and isValid
, returning isValid
and then assigning the primary scopes validity state to that as the loop happens. That would complicate the code much more for people trying to maintain in later then what the current setup provides.
I'm looking into adding more test coverage now, but that is the update on the code style. If anyone has an idea on a pattern to let the inner function get moved out without overcomplicating utilizing it then I'm all ears.
@@ master #65 diff @@
===================================
Files 3 3
Lines 94 121 +27
Methods 0 0
Messages 0 0
Branches 0 0
===================================
+ Hits 94 121 +27
Misses 0 0
Partials 0 0
Powered by Codecov. Last update 04facc4...3149921
@kentcdodds should we create a new release for this?
That happens automatically ;-) Check the releases page on this repo, it's probably already done.
Learn more about how that works here :)
Whoops! Looks like my GitHub token and NPM token are busted... hold please.
Weird, it appears travis doesn't have a build for the latest version of master... 🤔
Thank you all! If anyone reports a bug with the scope checking (or wants it modified somehow) just ping me. I don't mind hopping back in to work on it.
Hey @Garbee, thanks so much for your help! I've added you as a collaborator on the project. Please make sure that you review the CONTRIBUTING.md file (specifically the bit about the commit messages and the git hooks) and familiarize yourself with the code of conduct (we're using the contributor covenant). You might also want to watch the repo to be notified when someone files an issue/PR. Please continue to make PRs as you feel the need (you can make your branches directly on the repo rather than your fork if you want). Thanks! And welcome to the team :)
@kentcdodds thank you very much for the link on writing open source!
Alrighty, this has now been released as the latest version. Sorry for the trouble earlier! Thanks again!
This adds support for validating allowed scopes. To do this we provide a
scopes
option object. It can contain one of four possible keys:required
- Boolean to determine if a scope is requiredmultiple
- Boolean to allow for multiple scopes or a singular scope onlyallowed
- Array of possible values or'*'
for anything.validate
- Boolean to turn on validationThis does currently use an ES6 arrow function. So, that may increase the required node version. I'm not sure what the minimum supported version is. If the arrow function is a problem then I can refactor this out.