Support for expanded opengraph metadata #140
Conversation
Codecov Report
@@ Coverage Diff @@
## master #140 +/- ##
=======================================
Coverage 87.78% 87.78%
=======================================
Files 11 11
Lines 475 475
Branches 103 103
=======================================
Hits 417 417
Misses 52 52
Partials 6 6
Continue to review full report at Codecov.
|
lopuhin
left a comment
lopuhin
left a comment
There was a problem hiding this comment.
Thanks @ShivinDass left some minor comments, but most importantly, I wonder if an easier fix is possible here, please check the last comment.
lopuhin
left a comment
lopuhin
left a comment
There was a problem hiding this comment.
Looks great, thank you @ShivinDass . I think PR is almost ready to be merged, could you please check a few minor formatting suggestions?
| 'profile': 'http://ogp.me/ns/profile#' | ||
| }) | ||
|
|
||
|
|
There was a problem hiding this comment.
A minor cosmetic request: could you please revert unrelated changes (removed lines) here and below - e.g. here two line lines between top-level statements are used, following pep8.
There was a problem hiding this comment.
@lopuhin I'm not too familiar with pep8 but I've made the changes to the best of my understanding. Please do let me know if you find anything else out of order :)
There was a problem hiding this comment.
Looks great, thanks @ShivinDass ! By pep-8 I meant https://www.python.org/dev/peps/pep-0008/ but here I mostly wanted to ensure that PR has formatting consistent with current code and no unrelated changes, thank you!
lopuhin
left a comment
lopuhin
left a comment
There was a problem hiding this comment.
Looks great, thanks @ShivinDass ! Will merge if no one wants to do another review.
3 participants
Fixes #31
The merge would result in identifying expanded opengraph metadata properties and replacing them with their corresponding namespaces by adding appropriate prefix to the html tag