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

Use cache-dependency-path in actions/setup-go #3911

Closed
pellared opened this issue Mar 20, 2023 · 8 comments
Closed

Use cache-dependency-path in actions/setup-go #3911

pellared opened this issue Mar 20, 2023 · 8 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed pkg:tooling Related to the tooling package

Comments

@pellared
Copy link
Member

pellared commented Mar 20, 2023

With actions/setup-go updated to v4 the caching is enabled by default. We should consider setting cache-dependency-path: '**/go.sum'. However, we need to ensure on a fork what is time diff benefit (if any).

Originally posted by @pellared in #3901 (comment)

@MrAlias MrAlias added help wanted Extra attention is needed good first issue Good for newcomers pkg:tooling Related to the tooling package labels Mar 20, 2023
@tijmenstor
Copy link
Contributor

tijmenstor commented Mar 21, 2023

Hey, I'd like to test the time benefit of this if possible.

@tijmenstor
Copy link
Contributor

tijmenstor commented Mar 28, 2023

@MrAlias & @pellared comparing current runs to new runs using cache-dependency-path, most notable benefit is MacOS compatibility test runs. Overall, adding this specifically does not give a lot of benefit. You could only change the caching of MacOS for example.

old caching run

cache-dependency-path run

@ashutosh887
Copy link

@pellared @Tijmen34
Please assign me to work on this issue

@dmathieu
Copy link
Member

dmathieu commented May 2, 2023

Is there really any work needed? It seems @Tijmen34's test showed this has very little impact.

@tijmenstor
Copy link
Contributor

I actually do see some value in adding this to improve the speed of MacOS compatibility tests. It is however up for debate.

@pellared
Copy link
Member Author

pellared commented May 8, 2023

@Tijmen34 I really like this commit: https://github.com/Tijmen34/opentelemetry-go/commit/22221455677144d26173711552c314e4206c157c

  1. It is simpler
  2. It is a little faster (for sure not worse)
  3. It has been recently showcased in https://github.com/mvdan/github-actions-golang/ per Update setup-go mvdan/github-actions-golang#27

Feel free to open a PR 😉

@tijmenstor
Copy link
Contributor

@Tijmen34 I really like this commit: Tijmen34@2222145

1. It is simpler

2. It is a little faster (for sure not worse)

3. It has been recently showcased in https://github.com/mvdan/github-actions-golang/ per [Update setup-go mvdan/github-actions-golang#27](https://github.com/mvdan/github-actions-golang/pull/27)

Feel free to open a PR 😉

Thanks, great to hear. PR has been opened, very happy that I have been able to contribute. 😄

MrAlias added a commit that referenced this issue May 9, 2023
… ci workflow (#4074)

Using cache-dependency-path (which is now a built-in functionality in actions/setup-go), the speed of runs on MacOS can be improved.

Refs: #3911

Co-authored-by: Robert Pająk <[email protected]>
Co-authored-by: Chester Cheung <[email protected]>
Co-authored-by: Tyler Yahn <[email protected]>
@pellared
Copy link
Member Author

Fixed in #4074

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed pkg:tooling Related to the tooling package
Projects
None yet
Development

No branches or pull requests

5 participants