-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add required macros check #215
Add required macros check #215
Conversation
@McGwire-Jones - many thanks for this! I'll ask one of our developers to review it. In the meantime, would you mind expanding the README section to include an example, to help others who may try to use the feature? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @McGwire-Jones,
Thanks for your contribution!
Your PR looks great in general, but a few minor tweaks are required.
Than you @YuryShkoda for the feedback! I've made the changes you requested. |
Failing on the lint check - can you please run EDIT- you addressed in the meantime, thanks |
so this got released to NPM: https://www.npmjs.com/package/@sasjs/lint But not github, due to expired token. looking into it |
@all-contributors please add @McGwire-Jones for code |
I've put up a pull request to add @McGwire-Jones! 🎉 |
Issue
Link any related issue(s) in this section.
Intent
This PR is meant to add a new option that allows the user to enforce required macro options.
Implementation
A new file based rule hasRequiredMacroOptions has been added that checks the options a macro is defined with to look for the options listed in requiredMacroOptions.
The LintConfig has been updated to check for hasRequiredMacroOptions and if it is set to true to read a list of strings from requiredMacroOptions.
The readMe has been updated to mention this new option and its default values.
Checks
npm run lint:fix
).npm test
).