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
New: allows plugins to specify the URLs to their docs (fixes #6582) #6589
Conversation
Thanks for the pull request, @pmcelhaney! I took a look to make sure it's ready for merging and found some changes are needed:
Can you please update the pull request to address these? (More information can be found in our pull request guide.) |
Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA). 📝 Please visit http://contribute.jquery.org/CLA/ to sign. After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know. If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check. |
}; | ||
``` | ||
|
||
|
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.
Lots of useless whitespace. I'll clean this up.
module.exports = { | ||
getDocURI: function(ruleName) { | ||
return "http://path.to/docs/" + ruleName; | ||
} |
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.
Thinking it may be useful to pass the problem object as a second argument so the plugin can link to more specific detail about how the rule was violated. An IDE could even fetch the URL and provide contextual help.
But that's another PR for another day.
2460799
to
d79cd1c
Compare
LGTM |
d79cd1c
to
7fbd017
Compare
LGTM |
thanks for the PR, @pmcelhaney! Adding the "do not merge" label for now while we discuss the details in #6582 |
Closing based on feedback from the team. |
This is a first pass at implementation. I'm putting it out now to get feedback. Will come back and write a proper commit message.