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

feat: Crawlee + Cheerio templates code refactoring + Readme update #172

Merged
merged 18 commits into from
Aug 14, 2023
Merged

feat: Crawlee + Cheerio templates code refactoring + Readme update #172

merged 18 commits into from
Aug 14, 2023

Conversation

HonzaTuron
Copy link
Contributor

Closes apify/apify-web#2669.

Removing routes.js, updating readme, code refactoring, correct typing.

@github-actions github-actions bot added this to the 68th sprint - Web team milestone Jul 26, 2023
@github-actions github-actions bot added the t-web Issues with this label are in the ownership of the web team. label Jul 26, 2023
jbartadev
jbartadev requested changes Jul 28, 2023
View reviewed changes
templates/js-crawlee-cheerio/.actor/input_schema.json Outdated Show resolved Hide resolved
templates/js-crawlee-cheerio/src/main.js Outdated Show resolved Hide resolved
templates/js-crawlee-cheerio/src/main.js Outdated Show resolved Hide resolved
templates/ts-crawlee-cheerio/.actor/input_schema.json Outdated Show resolved Hide resolved
templates/js-crawlee-cheerio/README.md Outdated Show resolved Hide resolved
templates/js-crawlee-cheerio/README.md Outdated Show resolved Hide resolved
templates/manifest.json Show resolved Hide resolved
HonzaTuron and others added 3 commits July 28, 2023 09:15
@HonzaTuron @jbartadev
Apply suggestions from code review
51411fa 
Co-authored-by: Jan Bárta <45016873+jbartadev@users.noreply.github.com>
@HonzaTuron
feat(app): cr objections
d700717
@HonzaTuron
feat(app): cr objections
aa9826b
monkey-denky
monkey-denky reviewed Jul 28, 2023
View reviewed changes
Copy link
Contributor

@monkey-denky monkey-denky left a comment

Choose a reason for hiding this comment

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

Looks good but the tests are failing :/

@jbartadev
Copy link
Member

Looks good but the tests are failing :/

@nguyeda1 @HonzaTuron
I think it's not the fault of this PR, Franta mentioned it here
#171 (comment)

jbartadev
jbartadev requested changes Aug 1, 2023
View reviewed changes
Copy link
Member

@jbartadev jbartadev left a comment

Choose a reason for hiding this comment

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

Nice, I have a couple of comments on the template description, that should be applied to both templates.

Also, adding @B4nan as a reviewer to make sure this is the template behavior we agreed on.

templates/ts-crawlee-cheerio/README.md Outdated Show resolved Hide resolved
templates/ts-crawlee-cheerio/README.md Outdated Show resolved Hide resolved
templates/ts-crawlee-cheerio/README.md Outdated Show resolved Hide resolved
templates/ts-crawlee-cheerio/README.md Outdated Show resolved Hide resolved
templates/ts-crawlee-cheerio/README.md Outdated Show resolved Hide resolved
@jbartadev jbartadev requested a review from B4nan August 1, 2023 10:03
HonzaTuron and others added 3 commits August 1, 2023 12:34
@HonzaTuron @jbartadev
Apply suggestions from code review
18c2b7d 
Co-authored-by: Jan Bárta <45016873+jbartadev@users.noreply.github.com>
@HonzaTuron @jbartadev
Update templates/ts-crawlee-cheerio/README.md
1aa0fd6 
Co-authored-by: Jan Bárta <45016873+jbartadev@users.noreply.github.com>
@HonzaTuron
fix cr objections
6f74a68
jbartadev
jbartadev approved these changes Aug 2, 2023
View reviewed changes
Copy link
Member

@jbartadev jbartadev left a comment

Choose a reason for hiding this comment

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

Looks good to me, now it's on @B4nan

B4nan
B4nan requested changes Aug 9, 2023
View reviewed changes
Copy link
Member

@B4nan B4nan left a comment

Choose a reason for hiding this comment

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

left some comments on the JS template - they seem to be all valid for the TS template as well

templates/js-crawlee-cheerio/README.md Outdated Show resolved Hide resolved
templates/js-crawlee-cheerio/README.md Outdated Show resolved Hide resolved
templates/js-crawlee-cheerio/src/main.js Outdated Show resolved Hide resolved
templates/js-crawlee-cheerio/src/main.js Outdated Show resolved Hide resolved
templates/ts-crawlee-cheerio/src/main.ts Outdated Show resolved Hide resolved
HonzaTuron and others added 2 commits August 9, 2023 10:50
@HonzaTuron @B4nan
Apply suggestions from code review
58438bd 
Co-authored-by: Martin Adámek <banan23@gmail.com>
@HonzaTuron
Fix PR objections
07de88e
@HonzaTuron HonzaTuron requested a review from B4nan August 9, 2023 09:58
@HonzaTuron HonzaTuron requested a review from B4nan August 9, 2023 11:05
@HonzaTuron @B4nan
Update templates/ts-crawlee-cheerio/src/main.ts
a165ab9 
Co-authored-by: Martin Adámek <banan23@gmail.com>
@HonzaTuron HonzaTuron requested a review from B4nan August 14, 2023 14:16
@HonzaTuron HonzaTuron merged commit 334dc03 into master Aug 14, 2023
10 of 14 checks passed
@HonzaTuron HonzaTuron deleted the feat/crawlee-cheerio branch August 14, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@B4nan B4nan B4nan approved these changes

@monkey-denky monkey-denky monkey-denky approved these changes

@jbartadev jbartadev jbartadev approved these changes

Assignees

@HonzaTuron HonzaTuron

Labels
t-web Issues with this label are in the ownership of the web team.
Projects
None yet
Milestone
68th sprint - Web team
Development

Successfully merging this pull request may close these issues.
4 participants
@HonzaTuron @jbartadev @B4nan @monkey-denky