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

Add a callback on Loader to signal when a Load starts #1580

Merged
merged 3 commits into from
Aug 20, 2024

Conversation

colinkho
Copy link
Contributor

@colinkho colinkho commented Aug 1, 2024

This includes variables that captures the error and error count if it is starting a retry

@microkatz microkatz assigned microkatz and icbaker and unassigned microkatz Aug 7, 2024
@colinkho
Copy link
Contributor Author

colinkho commented Aug 7, 2024

@icbaker I have incorporated the comments

@icbaker
Copy link
Collaborator

icbaker commented Aug 13, 2024

I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!

@colinkho
Copy link
Contributor Author

colinkho commented Aug 15, 2024

Apologies @icbaker , I accidentally updated my main branch and had newer commits rebased on top. Hope this doesn't complicate the internal review

@icbaker
Copy link
Collaborator

icbaker commented Aug 16, 2024

No worries, i've force pushed my own changes back.

@icbaker
Copy link
Collaborator

icbaker commented Aug 16, 2024

btw I think people often send PRs from not the main branch of their forks, possibly for this reason, but as an aside: using the main branch in your fork also slightly upsets GitHub's command line tool because we have to run:

$ gh pr checkout 1580 --branch pr-1580

Because the regular command will try to update our main branch directly, which we don't want (and often fails anyway):

$ gh pr checkout 1580

So for future PRs, it might be useful to create a new well-named branch in your fork for each one.

@colinkho
Copy link
Contributor Author

Thanks. I didn't realize that the branch of the fork continuously updates the PR. I had thought it took a snapshot or merely cherry-picked the commits.

Also good info on the gh command line. I've been doing things the old-fashioned git way -- I'll try this out. Thank you!

@copybara-service copybara-service bot merged commit 24f3856 into androidx:main Aug 20, 2024
1 check passed
@androidx androidx locked and limited conversation to collaborators Oct 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants