Skip to content

Conversation

@optic5471ms
Copy link
Contributor

Updates privilege names to be more clear, and adds privileges to closures.

  • The "read_only" privilege is now named "restricted_execution". Updated ts and msdoc generators to use new name in documentation generation
  • The "none" privilege is now named "default". No ts or msdoc generators needed updates for this, we didnt output this information previously
  • Closures now support the privilege that they would be executed with in the metadata input for the inputs under the "call_privilege" member. Closures support 1 privilege type, so this is not an array like property and function privileges.
    • msdocs and ts generators now output the closure privilege of properties, function arguments, and function returns as additional comments for the type
  • Added tests for renaming privileges, backwards compatibility supported through upgrading to the new name
  • Added tests for closure privileges, backwards compatibility supported through just not outputting remarks on closures without their privilege tags.
  • Added privilege enum with the various privilege strings to use throughout generators now. Prevents errors when trying strings manually.

Copy link
Contributor

@SBLMikeDemone SBLMikeDemone left a comment

Choose a reason for hiding this comment

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

Looks good to me!

readonly closureNoPrivilegeShouldntBreak: () => void;
/**
* @remarks
* This closure is called with default privilege.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not wrong but can we assume default privilege and remove this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah we can definitely do that. Before I make the change, anyone else have opinions?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Mike, the default privilege comment is probably unnecessary.

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