Skip to content
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

changed the opengraph meta data extraction to incorporate the html body. #197

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

frostrot
Copy link

#192 Added the feature to incorporate all the meta tags outside of the html head, by changing in the function extract_items() in class openClassExtractor. Furthermore, added a test case to named opengraph_test_2 which uses the html of https://www.youtube.com/c/Freecodecamp where the meta tags are also present in the body of the html, and the function is able to correctly identify all the tags and parse it.

Copy link
Member

@lopuhin lopuhin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @frostrot , that's a nice improvement, and thanks for adding tests. Still have a few things left to review. However, would you mind reverting changes unrelated to this PR, such as changing single quotes to double quotes, etc.? This would make it easier to track when and why certain parts of the code were changed.

@lopuhin
Copy link
Member

lopuhin commented May 16, 2022

@frostrot also would you mind checking test failures? They look to be related to this PR.

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.

None yet

2 participants