Skip to content

Try to cache the clean version of flutter Dartdoc uses #2802

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

Merged
merged 5 commits into from
Sep 21, 2021

Conversation

jcollins-g
Copy link
Contributor

@jcollins-g jcollins-g commented Sep 20, 2021

Perhaps fixes the infrastructure part of #2799.

This could speed up most runs for the flutter bot by a few minutes. The cache is very conservative, being sure to throw away once a day just in case of accumulated cruft.

Uncached run: https://github.com/dart-lang/dartdoc/pull/2802/checks?check_run_id=3665189847

Cached run: https://github.com/dart-lang/dartdoc/pull/2802/checks?check_run_id=3665918707

@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Sep 20, 2021
@jcollins-g
Copy link
Contributor Author

Definitely improved the runtime, so mailing out.

@jcollins-g jcollins-g requested a review from athomas September 21, 2021 17:20
@jcollins-g
Copy link
Contributor Author

I will go ahead and land this as speeding up these bots has immediate, noticeable benefits. But @athomas if you have some comments or suggestions on how to improve this, I'll implement in a follow up.

@jcollins-g jcollins-g merged commit 8f7a20c into dart-lang:master Sep 21, 2021
@jcollins-g jcollins-g deleted the cache-pub-clean-flutter branch September 21, 2021 21:41
@athomas
Copy link
Member

athomas commented Sep 22, 2021

LGTM, perhaps the .pub-cache cache could also be used on other jobs than flutter (not sure if it should be shared between jobs, probably not across OSs at least so the key would need to include the OS).

You may also want to upgrade setup-dart to 1.2 (there's some fix with git dependencies on Windows in that version).

What happened to the coverage, did that stop working?

@jcollins-g
Copy link
Contributor Author

@athomas WRT .pub-cache, yeah I know I don't want the flutter cache sharing the others as Flutter needs far more pub packages than the main test. The others using a Flutter-test generated cache though would be fine. Not sure how to make GitHub Actions do that.

I'll look into updating setup-dart. I disabled coverage because it kept breaking and I wasn't getting enough benefit from coverage for it to be worth the time I was investing in the infrastructure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants