Detect authors in RDFa in addition to Microdata#9
Conversation
ThomasGreiner
left a comment
ThomasGreiner
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
Note that the author detection is currently only used to determine its potential impact and is therefore only limited to youtube.com. It'll come in handy later on though to have RDFa support.
src/lib/content/stats/author.js
Outdated
There was a problem hiding this comment.
Coding style: Make sure to run npm run lint (see also README) so that it can let you know about any coding style issues it finds. In this case, for instance, it'd point out that the line is longer than the 80 characters we defined.
There was a problem hiding this comment.
Make sure to run
npm run lint(see also README)
This should be in CONTRIBUTING. It would pop-up whenever someone opens a pull-request.
There was a problem hiding this comment.
You're right and it's already on our list of to-dos to finish open sourcing the extension (along with creating a CHANGES.md file and moving existing documentation over to the GitHub Wiki). So I'm sorry about the lack of those for now.
There was a problem hiding this comment.
circleci should automatically run our tests (including the linters) for pull requests now, so you won't have to run it locally if you don't want to, or forget to. Once an update is pushed to this pull request it should run for this pr.
src/lib/content/stats/author.js
Outdated
There was a problem hiding this comment.
Coding style: Please add a space before the second selector. Unfortunately, the linter doesn't catch that automatically.
Alos switch to case-insensitive matching.
ThomasGreiner
left a comment
ThomasGreiner
left a comment
There was a problem hiding this comment.
LGTM
Now the only thing that's left is to fill out our contributor's license agreement and send it back to us (e.g. via email to t.greiner@eyeo.com). That step is necessary for us to be able to use your code.
|
@da2x I just quickly wanted to check back with you regarding the contributor's license agreement I mentioned in my previous comment. Mind sending that over so that we can merge your change? |
|
I’ll not sign any agreement with any company over three lines of code. However, I’ll make it easy on you: I hereby waive all copyright or other legal claims on commit c77a514. (Independently archived copy.) |
|
You must add a CONTRIBUTING.md file on this repo with your Guidelines and CLA (or a link to) These tools may be useful too : |
|
We're currently investigating various options together with our legal team on how we want to simplify this process including cla-assistant.io as well as DCOs. I've created #32 for making the according changes as soon as we got approval from them. Both of those methods seem to require the creation of a new pull request though. I've also forwarded the comment with the waiver to legal and am waiting for their response. Note that it would only apply to this one contribution though, whereas the CLA applies to all contributions. |
3 participants
Case-insensitive matching.