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

Update sanctuary-scripts to version 1.5.0 #5

Merged
merged 1 commit into from
May 1, 2018

Conversation

Avaq
Copy link
Owner

@Avaq Avaq commented May 1, 2018

No description provided.

@Avaq
Copy link
Owner Author

Avaq commented May 1, 2018

@davidchambers could you lend a hand?

This code example:

permissionary/index.js

Lines 67 to 94 in 5cd9614

//. ```js
//. // This defines a mapping from roles to permissions.
//. // We can use wildcards to assign multiple permissions at once.
//. > var createVerifier = checkPermission({
//. . 'content-reader': ['content.read', 'images.read'],
//. . 'content-writer': ['content.write', 'images.upload'],
//. . 'superadmin': ['*']
//. . })
//.
//. // Let's say our user Bob is a content-reader, and also a content-writer.
//. > var canBob = createVerifier(['content-reader', 'content-writer'])
//.
//. // And Alice is an administrator.
//. > var canAlice = createVerifier(['superadmin'])
//.
//. // Bob has this permission through his content-reader role.
//. > canBob('content.read')
//. true
//.
//. // Bob does not have this permission.
//. > canBob('users.create')
//. false
//.
//. // Alice, however, does. She has all permissions (even the ones
//. // we haven't thought of yet).
//. canAlice('users.create')
//. true
//. ```

...gets transformed to this for validation:

// This defines a mapping from roles to permissions.
// We can use wildcards to assign multiple permissions at once.
var createVerifier = checkPermission({
  'content-reader': ['content.read', 'images.read'],
  'content-writer': ['content.write', 'images.upload'],
  'superadmin': ['*']
});
;
// Let's say our user Bob is a content-reader, and also a content-writer.
var canBob = createVerifier(['content-reader', 'content-writer']);
;
// And Alice is an administrator.
var canAlice = createVerifier(['superadmin']);
;
// Bob has this permission through his content-reader role.
canBob('content.read');
true;

// Bob does not have this permission.
canBob('users.create');
false;

// Alice, however, does. She has all permissions (even the ones
// we haven't thought of yet).
canAlice('users.create')
true

...which causes linting errors for the no-extra-semi, semi-style, and semi rules.

Least clear to me is why the last statement doesn't have a semicolon.

@davidchambers
Copy link
Contributor

The interim fix is to disable these rules for *.md files as I did in fluture-js/fluture-sanctuary-types#26. The long-term fix is to update rewrite to insert semicolons more discerningly.

@Avaq
Copy link
Owner Author

Avaq commented May 1, 2018

The interim fix is to disable these rules for *.md files

I'm okay with that for now. I wanted to be sure I'm using transcribe correctly. I created sanctuary-js/sanctuary-scripts#14 as a reminder.

@Avaq Avaq force-pushed the greenkeeper/sanctuary-scripts-1.5.0 branch from 5cd9614 to 3aecd39 Compare May 1, 2018 12:02
@codecov-io
Copy link

codecov-io commented May 1, 2018

Codecov Report

Merging #5 into master will not change coverage.
The diff coverage is n/a.

@Avaq Avaq merged commit cae1368 into master May 1, 2018
@Avaq Avaq deleted the greenkeeper/sanctuary-scripts-1.5.0 branch May 1, 2018 12:08
@@ -40,7 +41,7 @@ assign both roles to that user.

## API

<h4 name="checkPermission"><code><a href="https://github.com/Avaq/permissionary/blob/v0.1.0/index.js#L49">checkPermission :: StrMap (Array String) -⁠> Array String -⁠> String -⁠> Boolean</a></code></h4>
#### <a name="checkPermission" href="https://github.com/Avaq/permissionary/blob/v0.0.0/index.js#L50">`checkPermission :: StrMap (Array String) -⁠> Array String -⁠> String -⁠> Boolean`</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

What happened to the version number? 😱

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oh darn it.. I committed the README I was generating locally to identify the issue. I should be more careful and less hurried. I will push a fix to master as if running sanctuary-release. >.<

Copy link
Owner Author

Choose a reason for hiding this comment

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

Actually I'll just release a patch for consistency.

@davidchambers
Copy link
Contributor

Least clear to me is why the last statement doesn't have a semicolon.

The leading is missing. ;)

@davidchambers
Copy link
Contributor

I have a fix ready for the missing , but we should resolve #10 first. ;)

@davidchambers
Copy link
Contributor

I just opened #11 to add the missing > .

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.

3 participants