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

Adds reviews as a possible output #1273

Closed
wants to merge 4 commits into from
Closed

Conversation

jknndy
Copy link
Collaborator

@jknndy jknndy commented Oct 5, 2024

Resolves #870

As title states this adds support for reviews to be gathered via schema. I'm not sure about how tests should be handled here so I included all the reviews for the 70 (up from 25 at the time of that issue opening!) JSONs but I think it could be simplified.

Possibly just gathering the oldest comment returned and included that in the test?

@jknndy jknndy marked this pull request as ready for review October 5, 2024 19:58
@jayaddison
Copy link
Collaborator

Although I filed the feature request: I have some concerns about supporting reviews. In particular, two of them are:

  • Reviews are user-generated content, and although we wouldn't be managing or storing it directly (other than in the test JSON expectations), I worry that there might be unexpected requirements/consequences for this library if we retrieve them. I don't know what those would be -- but since there hasn't yet been much demand for this feature, I'd suggest we try to keep things simple.
  • Slightly related: if someone wanted us to stop using their review content in our test cases -- which notably we don't distribute with releases, but still do use during testing -- we would have a tricky time confirming whether they're genuinely the person who is requesting removal (we don't host the website the review was posted to, don't have contact details, etc for them to confirm anything).

As a counter-argument, I can admit that there could be value for some users of this library in accessing reviews about recipes; they can sometimes provide valuable context, recommendations, ideas etc about the meal - so I think it is potentially a useful feature. But at the moment I don't feel I know enough about the potential risks, and there doesn't seem to be overwhelming demand for the feature -- hence my reluctance.

Even if demand does grow I'll probably still be a bit cautious about this, because we don't have a lot of resources (time/people/etc) to handle unexpected problems after-the-fact - hence another reason to keep most of the scraping we support focused on the recipe-related data written by the recipe author and/or publisher themselves.

@jknndy
Copy link
Collaborator Author

jknndy commented Oct 8, 2024

I agree with you. While it’s a nice addition in theory, the potential unintended consequences make me lean towards not including it at this time. I'll close this PR now!

@jknndy jknndy closed this Oct 8, 2024
@jknndy jknndy deleted the reviews branch October 8, 2024 20:40
@jayaddison
Copy link
Collaborator

Thanks @jknndy!

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.

Add retrieval of reviews by using schema.org metadata
2 participants