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.

Sign up for GitHub

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

Jump to bottom

Add support for kellyscleankitchen.com #1425

Merged
merged 6 commits into from
Jan 14, 2025
Merged

Add support for kellyscleankitchen.com #1425

merged 6 commits into from
Jan 14, 2025

Conversation

ashiramin
Copy link
Contributor

@ashiramin ashiramin commented Dec 8, 2024 •
edited
Loading

Resolves #1426

@ashiramin ashiramin marked this pull request as ready for review December 8, 2024 04:29
@jknndy
Copy link
Collaborator

jknndy commented Dec 9, 2024 •
edited
Loading

Hi @ashiramin , thanks for the contribution!
Before we can validate this is functioning as expected you'll need to add the JSON counterpart to the testhtmls you provided (docs).

edit: Only 1 set of testhtml & JSON is required, unless implementing ingredient_groups functionality.

@ashiramin
Copy link
Contributor Author

@jknndy I've made the updates. Please let me know what you think

jknndy
jknndy requested changes Dec 11, 2024
View reviewed changes
Copy link
Collaborator

@jknndy jknndy left a comment

Choose a reason for hiding this comment

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

A few comments throughout but overall looks good! You'll also need to add test coverage for description, cook_time and prep_time and then we should be good to go.

recipe_scrapers/__init__.py Outdated Show resolved Hide resolved
recipe_scrapers/kellyscleankitchen.py Outdated Show resolved Hide resolved
recipe_scrapers/kellyscleankitchen.py Outdated Show resolved Hide resolved
recipe_scrapers/kellyscleankitchen.py Outdated Show resolved Hide resolved
recipe_scrapers/kellyscleankitchen.py Outdated Show resolved Hide resolved
recipe_scrapers/kellyscleankitchen.py Outdated Show resolved Hide resolved
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@ashiramin ashiramin force-pushed the add-support-for-kellyscleankitchen branch from d8de5a8 to 1c89707 Compare January 5, 2025 20:43
@ashiramin
Add support for kellyscleankitchen.com
a54e319
@ashiramin
Add test json file
aded598
@ashiramin
Update tests
303ae71
@ashiramin
Update test and code based on feedback
b64d3cf 
Update recipe_scrapers/kellyscleankitchen.py

Co-authored-by: Joey <7505194+jknndy@users.noreply.github.com>

Update recipe_scrapers/kellyscleankitchen.py

Co-authored-by: Joey <7505194+jknndy@users.noreply.github.com>

Update recipe_scrapers/kellyscleankitchen.py

Co-authored-by: Joey <7505194+jknndy@users.noreply.github.com>

Update recipe_scrapers/kellyscleankitchen.py

Co-authored-by: Joey <7505194+jknndy@users.noreply.github.com>

Update recipe_scrapers/kellyscleankitchen.py

Co-authored-by: Joey <7505194+jknndy@users.noreply.github.com>

Update test coverage
@ashiramin ashiramin force-pushed the add-support-for-kellyscleankitchen branch from 1c89707 to b64d3cf Compare January 5, 2025 21:02
@ashiramin ashiramin requested a review from jknndy January 5, 2025 21:03
@ashiramin
Copy link
Contributor Author

@jknndy finally got around to making the changes you requested. Let me know what you think

@jknndy
Copy link
Collaborator

jknndy commented Jan 5, 2025

Looks good to me, thanks again for the contribution & happy new year. I'll leave this open for a few days for any other reviews and then merge!

@jknndy
Copy link
Collaborator

jknndy commented Jan 7, 2025

@ashiramin , I apologize but there is one more thing to fix! While this PR was open the supported sites list was moved from the readme into a web hosted doc which is procedural generated on merge. Could you remove your changes to README.rst? If you're unable to I can push a commit myself to fix it. Sorry about that!

@ashiramin
Copy link
Contributor Author

@ashiramin , I apologize but there is one more thing to fix! While this PR was open the supported sites list was moved from the readme into a web hosted doc which is procedural generated on merge. Could you remove your changes to README.rst? If you're unable to I can push a commit myself to fix it. Sorry about that!

@jknndy all good made the updates.

jknndy
jknndy approved these changes Jan 9, 2025
View reviewed changes
Copy link
Collaborator

@jknndy jknndy left a comment

Choose a reason for hiding this comment

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

Thanks again!

@hhursev hhursev merged commit 3301d0e into hhursev:main Jan 14, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jknndy jknndy jknndy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for kellyscleankitchen
3 participants
@ashiramin @jknndy @hhursev