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

docs: refactor crawling context helper methods to have them properly documented #1550

Merged
merged 3 commits into from
Sep 21, 2022

Conversation

B4nan
Copy link
Member

@B4nan B4nan commented Sep 19, 2022

  • simplifies enqueueLinks implementation - no more child types for options
  • improves types of context properties, e.g. crawler has correct type in browser crawlers
  • adds jsdoc for context helpers, e.g. injectJQuery or sendRequest
  • all context helpers have their "jsdoc copy", we could mauybe use @inheritDoc as an inline tag pointing to the implementation, but that would mean taking the same jsdoc - here we instead have customized version, that dont have the page param (as its context bound) and have context related examples
  • contains unrelated changes in the SystemStatus to support edge case i found, we can move this outside of the PR but its not really worth having a changelog line on its own :]

@B4nan B4nan force-pushed the cleanup-context-helpers branch 5 times, most recently from 99838da to 4775f1b Compare September 19, 2022 18:52
@B4nan B4nan force-pushed the cleanup-context-helpers branch 2 times, most recently from 3f13f9f to deda5de Compare September 19, 2022 21:27
@B4nan B4nan requested a review from vladfrangu September 20, 2022 11:04
@B4nan B4nan marked this pull request as ready for review September 20, 2022 11:08
@B4nan B4nan force-pushed the cleanup-context-helpers branch from f145f15 to 6b205e2 Compare September 21, 2022 13:59
@B4nan B4nan merged commit 35b717b into master Sep 21, 2022
@B4nan B4nan deleted the cleanup-context-helpers branch September 21, 2022 14:08
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.

2 participants