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

Update Polygon.js for 2018 #1253

Merged
merged 3 commits into from
May 26, 2021
Merged

Update Polygon.js for 2018 #1253

merged 3 commits into from
May 26, 2021

Conversation

owcz
Copy link
Contributor

@owcz owcz commented Mar 11, 2017

authors was broken in prior format, rewritten in FW

@owcz
Copy link
Contributor Author

owcz commented Mar 11, 2017

Some of the site's old "feature" articles (e.g., this one) use non-standard metadata (particularly re: author). Not sure how best to handle them—open to suggestions, or just ignoring them (they're not compatible with old translator either)

@adam3smith
Copy link
Collaborator

I haven't looked at the differences in depth, but this is the type of things you'd find much easiert to address in a non-FW translator, where you could just cycle through a couple of ways of getting an author in turn. Given that you already have the existing structure, it'd makes sense to me to keep the old translator and just swap out the xpaths, then add checks for the other author options.

@owcz
Copy link
Contributor Author

owcz commented Mar 11, 2017

The site's early Features (from several years ago) have inconsistent formatting, hence why I'm not particularly concerned about handling the edge cases. (FW handles 99% of the site's articles and is much easier to maintain.)

@adam3smith Is there no way to do a secondary detection when FW.Scraper runs for "creators" and finds nothing? There is a fairly standard //meta[@property="author"]/@content field, but it will only show the first author if there are multiple (hence why I opted for c-byline__item instead).

@adam3smith
Copy link
Collaborator

I think you could do a hook testing for an empty creators array and using the other options if you want to stick to FW -- I find it quickly gets easier to write regular translators once you start using hooks, but up to you.

@owcz owcz changed the title Make Polygon.js current for 2017 Update Polygon.js for 2018 Jul 7, 2018
@owcz
Copy link
Contributor Author

owcz commented Jul 7, 2018

ready for review

@AbeJellinek
Copy link
Member

Squashed, linted, updated (a little bit), and merged! Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants