-
Notifications
You must be signed in to change notification settings - Fork 141
docs: Improvements of PPE docs #1875
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, please have a team admin upgrade your team to Bugbot Pro by visiting the Cursor dashboard. Your first 14 days will be free!
Comment @cursor review or bugbot run to trigger another review on this PR
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
metalwarrior665
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a great step forward. I added a few comments to resolve.
832beb5 to
951b444
Compare
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
mhamas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general looks good, thanks! I've left a couple of comments, please, have a look. Thanks!
sources/platform/actors/publishing/monetize/pricing_and_costs.mdx
Outdated
Show resolved
Hide resolved
sources/platform/actors/publishing/monetize/pricing_and_costs.mdx
Outdated
Show resolved
Hide resolved
sources/platform/actors/publishing/monetize/pricing_and_costs.mdx
Outdated
Show resolved
Hide resolved
|
Preview for this PR was built for commit |
Co-authored-by: Michał Olender <[email protected]>
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
|
@metalwarrior665, @mhamas, @TC-MO - I propose to merge it, and solve the open discussion later (I can create follow up issues). What do you think? |
metalwarrior665
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few small comments but I agree that we can relase it and iterate on the pending comments later.
b76afbc to
347a605
Compare
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
Co-authored-by: Michał Olender <[email protected]>
Co-authored-by: Michał Olender <[email protected]>
Co-authored-by: Michał Olender <[email protected]>
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
TC-MO
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great work, and not an easy task!
The following PR improves PPE documentation:
Related:
TODOs:
get_charged_event_countmethod similar to Apify JavaScript SDK (getChargedEventCount). I am discussing with the tooling team about the correct implementation in Actor.get_charged_event_countmethod to the Python SDK.