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

Install tools instead of relying on go run #241

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

chrisdoherty4
Copy link
Member

@chrisdoherty4 chrisdoherty4 commented Mar 15, 2023

go running tools is slow as Go recompiles the program with ever invocation. This PR changes to installing tools into $PWD/out/tools ready to be used by the rest of the Makefile improving the dependent recipe's performance.

@chrisdoherty4 chrisdoherty4 force-pushed the feature/tools.go branch 2 times, most recently from 623371b to 4180d86 Compare March 15, 2023 02:39
@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Merging #241 (f216aa7) into main (75ec45a) will not change coverage.
The diff coverage is n/a.

❗ Current head f216aa7 differs from pull request most recent head afc9083. Consider uploading reports for the commit afc9083 to get more accurate results

@@           Coverage Diff           @@
##             main     #241   +/-   ##
=======================================
  Coverage   76.21%   76.21%           
=======================================
  Files          17       17           
  Lines         412      412           
=======================================
  Hits          314      314           
  Misses         87       87           
  Partials       11       11           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@chrisdoherty4 chrisdoherty4 force-pushed the feature/tools.go branch 2 times, most recently from dacf33f to 54888da Compare March 15, 2023 02:46
@chrisdoherty4
Copy link
Member Author

chrisdoherty4 commented Mar 15, 2023

WIP: Need to work out the best way to exclude the tools.go.

@chrisdoherty4 chrisdoherty4 marked this pull request as draft March 16, 2023 18:43
@chrisdoherty4 chrisdoherty4 force-pushed the feature/tools.go branch 2 times, most recently from 58d7bc0 to 20ef31b Compare March 17, 2023 02:16
@chrisdoherty4 chrisdoherty4 changed the title Use go.mod to manage Go tooling refactor: install tools instead of relying on go run Mar 17, 2023
@chrisdoherty4 chrisdoherty4 marked this pull request as ready for review March 17, 2023 02:22
@chrisdoherty4 chrisdoherty4 force-pushed the feature/tools.go branch 3 times, most recently from d076039 to d311dbc Compare March 17, 2023 02:33
@chrisdoherty4 chrisdoherty4 added the ready-to-merge Signal to Mergify to merge the PR. label Mar 17, 2023
Makefile Outdated Show resolved Hide resolved
@chrisdoherty4 chrisdoherty4 changed the title refactor: install tools instead of relying on go run Install tools instead of relying on go run Mar 26, 2023
@chrisdoherty4 chrisdoherty4 force-pushed the feature/tools.go branch 2 times, most recently from 3d996e9 to 81464a6 Compare March 26, 2023 02:30
go run has proven sluggish when invoking Make recipes. This change
trends us toward an install and use approach skipping the compilation
invoked with go run.

Signed-off-by: Chris Doherty <chris.doherty4@gmail.com>
@mergify mergify bot merged commit df72ddf into tinkerbell:main Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Signal to Mergify to merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants