search

onetsp / RecipeParser

A PHP library for parsing structured recipe data from HTML files.
https://onetsp.com/
MIT License
93 stars 26 forks source link

Core 919: fix recipe parser for foodandwine #19

Closed OldGareBear closed 8 years ago

OldGareBear commented 9 years ago

Change Summary

Changed the XPath queries and other scraper logic and wrote a new routine for parsing out time in minutes based on mixed-time-unit strings. Updated HTML test files, updated existing scraper tests, and added a new test.

Release Notes

None.

Testing

Wrote unit tests for three recipes that represent all the variation in recipe structure that I could find (after checking about a dozen recipes). All tests are passing.

onetsp commented 9 years ago

Hi! Thanks for the PR for RecipeParser. I'd love to get these changes merged.

Could you tell me a bit more about what's going on here? I saw updates to the FoodandWine parser, along with related unit tests. I also saw a new Recipage parser, but I don't see tests for that. I'd prefer not to merge new parsers unless we have associated tests for them.

This is my first awareness of Recipage and took a brief look at it on danicasdaily.com. I'm assuming the point of this new parser is to be able to scrape recipes from sites that use your plug-in? Can we include tests against HTML from one of these sites that are using your plug-in?

Mike

OldGareBear commented 9 years ago

Hi Mike,

Yeah, I actually made a git mistake here! I only meant to make a pull request against onetsp with the commits related to FoodandWine. The other changes were made against my team's fork by another contributor, so I'm not actually familiar with them. That author of the those changes is actually out this week, so when he gets back I can discuss it with him and see if we can add some tests, so we can touch base then. Does that sound OK?

Thanks, Gary

On Sun, Sep 27, 2015 at 4:12 PM, Onetsp. notifications@github.com wrote:

Hi! Thanks for the PR for RecipeParser. I'd love to get these changes merged.

Could you tell me a bit more about what's going on here? I saw updates to the FoodandWine parser, along with related unit tests. I also saw a new Recipage parser, but I don't see tests for that. I'd prefer not to merge new parsers unless we have associated tests for them.

This is my first awareness of Recipage and took a brief look at it on danicasdaily.com. I'm assuming the point of this new parser is to be able to scrape recipes from sites that use your plug-in? Can we include tests against HTML from one of these sites that are using your plug-in?

Mike

— Reply to this email directly or view it on GitHub https://github.com/onetsp/RecipeParser/pull/19#issuecomment-143591550.

emilhdiaz commented 9 years ago

Hi Mike,

Sorry! I had to reorganize our fork so that these pull request work properly. I removed the Recipage code that I added originally as it is indeed not required.

Best, Emil

onetsp commented 8 years ago

Sorry for taking so long to merge this one. Thanks for the PR. :)