-
Notifications
You must be signed in to change notification settings - Fork 256
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
refactor(listGoFiles): remove go list dependency #440
refactor(listGoFiles): remove go list dependency #440
Conversation
I probably should've created an issue for this first 😓, please let me know if you'd like me to do that, otherwise I'm happy to chat about this on here or on the Slack! |
This sounds pretty great. I'm busy this weekend but I can take a look tomorrow. Thanks a lot for this proposal! |
Sorry to ping, but any update on a review? Totally understand if you're busy! |
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.
Other than a couple very minor additions, this looks great.
Co-authored-by: Nate Finch <[email protected]>
Co-authored-by: Nate Finch <[email protected]>
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 - with the updated meaningful error messages
FYI I'm going to be using |
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 looks really good. Thanks so much. Tried it on my local machine and it's definitely faster.
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.
reapprove
Thank you so much @natefinch! Is there a timeline I could get for releasing this? Eager to get this through |
I can make a release tonight:) |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/magefile/mage](https://togithub.com/magefile/mage) | require | minor | `v1.13.0` -> `v1.14.0` | --- ### Release Notes <details> <summary>magefile/mage</summary> ### [`v1.14.0`](https://togithub.com/magefile/mage/releases/tag/v1.14.0) [Compare Source](https://togithub.com/magefile/mage/compare/v1.13.0...v1.14.0) #### What's Changed - give props to netlify on the main page by [@​natefinch](https://togithub.com/natefinch) in [https://github.com/magefile/mage/pull/410](https://togithub.com/magefile/mage/pull/410) - Update site to build with newer hugo by [@​perrito666](https://togithub.com/perrito666) in [https://github.com/magefile/mage/pull/411](https://togithub.com/magefile/mage/pull/411) - bring docs for use of directives up to date by [@​efd6](https://togithub.com/efd6) in [https://github.com/magefile/mage/pull/407](https://togithub.com/magefile/mage/pull/407) - add blog post about v1.13 by [@​natefinch](https://togithub.com/natefinch) in [https://github.com/magefile/mage/pull/412](https://togithub.com/magefile/mage/pull/412) - fix author links by [@​natefinch](https://togithub.com/natefinch) in [https://github.com/magefile/mage/pull/413](https://togithub.com/magefile/mage/pull/413) - Add variadic support to mg.F by [@​perj](https://togithub.com/perj) in [https://github.com/magefile/mage/pull/402](https://togithub.com/magefile/mage/pull/402) - feat: rename templated imports to avoid collisions by [@​ladydascalie](https://togithub.com/ladydascalie) in [https://github.com/magefile/mage/pull/421](https://togithub.com/magefile/mage/pull/421) - Website: fixing typos by [@​deining](https://togithub.com/deining) in [https://github.com/magefile/mage/pull/429](https://togithub.com/magefile/mage/pull/429) - refactor(listGoFiles): remove go list dependency by [@​jaredallard](https://togithub.com/jaredallard) in [https://github.com/magefile/mage/pull/440](https://togithub.com/magefile/mage/pull/440) - doc: add MacPorts install info by [@​herbygillot](https://togithub.com/herbygillot) in [https://github.com/magefile/mage/pull/419](https://togithub.com/magefile/mage/pull/419) #### New Contributors - [@​efd6](https://togithub.com/efd6) made their first contribution in [https://github.com/magefile/mage/pull/407](https://togithub.com/magefile/mage/pull/407) - [@​perj](https://togithub.com/perj) made their first contribution in [https://github.com/magefile/mage/pull/402](https://togithub.com/magefile/mage/pull/402) - [@​ladydascalie](https://togithub.com/ladydascalie) made their first contribution in [https://github.com/magefile/mage/pull/421](https://togithub.com/magefile/mage/pull/421) - [@​deining](https://togithub.com/deining) made their first contribution in [https://github.com/magefile/mage/pull/429](https://togithub.com/magefile/mage/pull/429) - [@​jaredallard](https://togithub.com/jaredallard) made their first contribution in [https://github.com/magefile/mage/pull/440](https://togithub.com/magefile/mage/pull/440) - [@​herbygillot](https://togithub.com/herbygillot) made their first contribution in [https://github.com/magefile/mage/pull/419](https://togithub.com/magefile/mage/pull/419) **Full Changelog**: magefile/mage@v1.13.0...v1.14.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. Co-authored-by: mend-for-jackfan.us.kg[bot] <mend-for-jackfan.us.kg[bot]@users.noreply.github.com>
….0 (#6) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/magefile/mage](https://togithub.com/magefile/mage) | require | minor | `v1.11.0` -> `v1.14.0` | --- ### Release Notes <details> <summary>magefile/mage</summary> ### [`v1.14.0`](https://togithub.com/magefile/mage/releases/tag/v1.14.0): - Faster Than Ever [Compare Source](https://togithub.com/magefile/mage/compare/v1.13.0...v1.14.0) #### What's Changed - give props to netlify on the main page by [@​natefinch](https://togithub.com/natefinch) in [https://github.com/magefile/mage/pull/410](https://togithub.com/magefile/mage/pull/410) - Update site to build with newer hugo by [@​perrito666](https://togithub.com/perrito666) in [https://github.com/magefile/mage/pull/411](https://togithub.com/magefile/mage/pull/411) - bring docs for use of directives up to date by [@​efd6](https://togithub.com/efd6) in [https://github.com/magefile/mage/pull/407](https://togithub.com/magefile/mage/pull/407) - add blog post about v1.13 by [@​natefinch](https://togithub.com/natefinch) in [https://github.com/magefile/mage/pull/412](https://togithub.com/magefile/mage/pull/412) - fix author links by [@​natefinch](https://togithub.com/natefinch) in [https://github.com/magefile/mage/pull/413](https://togithub.com/magefile/mage/pull/413) - Add variadic support to mg.F by [@​perj](https://togithub.com/perj) in [https://github.com/magefile/mage/pull/402](https://togithub.com/magefile/mage/pull/402) - feat: rename templated imports to avoid collisions by [@​ladydascalie](https://togithub.com/ladydascalie) in [https://github.com/magefile/mage/pull/421](https://togithub.com/magefile/mage/pull/421) - Website: fixing typos by [@​deining](https://togithub.com/deining) in [https://github.com/magefile/mage/pull/429](https://togithub.com/magefile/mage/pull/429) - refactor(listGoFiles): remove go list dependency by [@​jaredallard](https://togithub.com/jaredallard) in [https://github.com/magefile/mage/pull/440](https://togithub.com/magefile/mage/pull/440) - doc: add MacPorts install info by [@​herbygillot](https://togithub.com/herbygillot) in [https://github.com/magefile/mage/pull/419](https://togithub.com/magefile/mage/pull/419) #### New Contributors - [@​efd6](https://togithub.com/efd6) made their first contribution in [https://github.com/magefile/mage/pull/407](https://togithub.com/magefile/mage/pull/407) - [@​perj](https://togithub.com/perj) made their first contribution in [https://github.com/magefile/mage/pull/402](https://togithub.com/magefile/mage/pull/402) - [@​ladydascalie](https://togithub.com/ladydascalie) made their first contribution in [https://github.com/magefile/mage/pull/421](https://togithub.com/magefile/mage/pull/421) - [@​deining](https://togithub.com/deining) made their first contribution in [https://github.com/magefile/mage/pull/429](https://togithub.com/magefile/mage/pull/429) - [@​jaredallard](https://togithub.com/jaredallard) made their first contribution in [https://github.com/magefile/mage/pull/440](https://togithub.com/magefile/mage/pull/440) - [@​herbygillot](https://togithub.com/herbygillot) made their first contribution in [https://github.com/magefile/mage/pull/419](https://togithub.com/magefile/mage/pull/419) **Full Changelog**: magefile/mage@v1.13.0...v1.14.0 ### [`v1.13.0`](https://togithub.com/magefile/mage/releases/tag/v1.13.0): - Magefiles Directory and more! [Compare Source](https://togithub.com/magefile/mage/compare/v1.12.1...v1.13.0) #### What's Changed - feat: allow mage:import alias to be defined for multiple imports by [@​viktorvoltaire](https://togithub.com/viktorvoltaire) in [https://github.com/magefile/mage/pull/398](https://togithub.com/magefile/mage/pull/398) - chore: wrap log.Println in cmd with verbose check by [@​viktorvoltaire](https://togithub.com/viktorvoltaire) in [https://github.com/magefile/mage/pull/399](https://togithub.com/magefile/mage/pull/399) - Detect deps that have already been invoked by [@​carolynvs](https://togithub.com/carolynvs) in [https://github.com/magefile/mage/pull/346](https://togithub.com/magefile/mage/pull/346) - Replace godoc.org URLs by [@​JamieEdge](https://togithub.com/JamieEdge) in [https://github.com/magefile/mage/pull/342](https://togithub.com/magefile/mage/pull/342) - Add magefiles directory support by [@​perrito666](https://togithub.com/perrito666) in [https://github.com/magefile/mage/pull/405](https://togithub.com/magefile/mage/pull/405) #### New Contributors - [@​viktorvoltaire](https://togithub.com/viktorvoltaire) made their first contribution in [https://github.com/magefile/mage/pull/398](https://togithub.com/magefile/mage/pull/398) - [@​carolynvs](https://togithub.com/carolynvs) made their first contribution in [https://github.com/magefile/mage/pull/346](https://togithub.com/magefile/mage/pull/346) - [@​JamieEdge](https://togithub.com/JamieEdge) made their first contribution in [https://github.com/magefile/mage/pull/342](https://togithub.com/magefile/mage/pull/342) - [@​perrito666](https://togithub.com/perrito666) made their first contribution in [https://github.com/magefile/mage/pull/405](https://togithub.com/magefile/mage/pull/405) **Full Changelog**: magefile/mage@v1.12.1...v1.13.0 ### [`v1.12.1`](https://togithub.com/magefile/mage/releases/tag/v1.12.1): - Second Verse, Same as the First [Compare Source](https://togithub.com/magefile/mage/compare/v1.12.0...v1.12.1) This is a copy of v1.12.0 ... nothing has changed. However, there was an initial v1.12.0 that was created accidentally, and then deleted, and it's causing some go proxies to complain. So hopefully this will fix that. #### Changelog - [`2f1ec40`](https://togithub.com/magefile/mage/commit/2f1ec40) ci: migrate from travis to github action ([#​391](https://togithub.com/magefile/mage/issues/391)) - [`fe9f942`](https://togithub.com/magefile/mage/commit/fe9f942) evidently goreleaseer has changed in the last 4 years :) ([#​394](https://togithub.com/magefile/mage/issues/394)) - [`fd5011e`](https://togithub.com/magefile/mage/commit/fd5011e) Fix the rollback mechanism for tags during a release ([#​392](https://togithub.com/magefile/mage/issues/392)) - [`404c119`](https://togithub.com/magefile/mage/commit/404c119) sh.run(): quoted strings before join ([#​306](https://togithub.com/magefile/mage/issues/306)) - [`0c5affe`](https://togithub.com/magefile/mage/commit/0c5affe) Add asdf installation instructions to docs ([#​383](https://togithub.com/magefile/mage/issues/383)) - [`e84bbc1`](https://togithub.com/magefile/mage/commit/e84bbc1) [#​288](https://togithub.com/magefile/mage/issues/288) add brew and scoop install to docs ([#​376](https://togithub.com/magefile/mage/issues/376)) - [`80953f7`](https://togithub.com/magefile/mage/commit/80953f7) [#​378](https://togithub.com/magefile/mage/issues/378) bump travisci go16 ([#​379](https://togithub.com/magefile/mage/issues/379)) - [`dd94424`](https://togithub.com/magefile/mage/commit/dd94424) Create issue templates ([#​374](https://togithub.com/magefile/mage/issues/374)) - [`d9e2e41`](https://togithub.com/magefile/mage/commit/d9e2e41) fix: deterministic compiled mainfile ([#​348](https://togithub.com/magefile/mage/issues/348)) - [`4cf3cfc`](https://togithub.com/magefile/mage/commit/4cf3cfc) make -h work with imported targets ([#​335](https://togithub.com/magefile/mage/issues/335)) - [`de7ca6c`](https://togithub.com/magefile/mage/commit/de7ca6c) fix test for go 1.16 ([#​330](https://togithub.com/magefile/mage/issues/330)) ### [`v1.12.0`](https://togithub.com/magefile/mage/releases/tag/v1.12.0): - Small Fixes [Compare Source](https://togithub.com/magefile/mage/compare/v1.11.0...v1.12.0) This is our first release in a while, and nothing major is added, but some small fixes have gone out, like making the mainfile deterministic and making sure we can use -h with imported targets. #### Changelog - [`2f1ec40`](https://togithub.com/magefile/mage/commit/2f1ec40) ci: migrate from travis to github action ([#​391](https://togithub.com/magefile/mage/issues/391)) - [`fe9f942`](https://togithub.com/magefile/mage/commit/fe9f942) evidently goreleaseer has changed in the last 4 years :) ([#​394](https://togithub.com/magefile/mage/issues/394)) - [`fd5011e`](https://togithub.com/magefile/mage/commit/fd5011e) Fix the rollback mechanism for tags during a release ([#​392](https://togithub.com/magefile/mage/issues/392)) - [`404c119`](https://togithub.com/magefile/mage/commit/404c119) sh.run(): quoted strings before join ([#​306](https://togithub.com/magefile/mage/issues/306)) - [`0c5affe`](https://togithub.com/magefile/mage/commit/0c5affe) Add asdf installation instructions to docs ([#​383](https://togithub.com/magefile/mage/issues/383)) - [`e84bbc1`](https://togithub.com/magefile/mage/commit/e84bbc1) [#​288](https://togithub.com/magefile/mage/issues/288) add brew and scoop install to docs ([#​376](https://togithub.com/magefile/mage/issues/376)) - [`80953f7`](https://togithub.com/magefile/mage/commit/80953f7) [#​378](https://togithub.com/magefile/mage/issues/378) bump travisci go16 ([#​379](https://togithub.com/magefile/mage/issues/379)) - [`dd94424`](https://togithub.com/magefile/mage/commit/dd94424) Create issue templates ([#​374](https://togithub.com/magefile/mage/issues/374)) - [`d9e2e41`](https://togithub.com/magefile/mage/commit/d9e2e41) fix: deterministic compiled mainfile ([#​348](https://togithub.com/magefile/mage/issues/348)) - [`4cf3cfc`](https://togithub.com/magefile/mage/commit/4cf3cfc) make -h work with imported targets ([#​335](https://togithub.com/magefile/mage/issues/335)) - [`de7ca6c`](https://togithub.com/magefile/mage/commit/de7ca6c) fix test for go 1.16 ([#​330](https://togithub.com/magefile/mage/issues/330)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/scottames/cmder). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS41Ny4wIiwidXBkYXRlZEluVmVyIjoiMzUuNTcuMCJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
What this PR does: This PR removes the usage of
go list
in favor of parsing the files with thego/build
package. This speeds up the discovery of go files from ms to us, which combined withMAGEFILE_HASHFAST
can reduce the time for a re-run ofmage
to ~200ms.I was getting a little irritated with every run of mage being at least 500-600ms due to
go list
taking a min of200ms
and being called twice, go list does a lot more then we were using it for. So, I propose we parse it instead.Notes for my reviewer: I did this pretty quickly, and without that much regard for style. I'm happy to split this up and clean it up if we're OK with the approach. Thank you for reviewing ✨
Related to #404
Performance Measurements
This PR:
$ MAGEFILE_HASHFAST=true hyperfine -i './bin/mage -debug -l' Benchmark 1: ./bin/mage -debug -l Time (mean ± σ): 182.7 ms ± 7.7 ms [User: 33.5 ms, System: 84.2 ms] Range (min … max): 171.9 ms … 199.5 ms 14 runs
1.13.0:
$ MAGEFILE_HASHFAST=true hyperfine -i "$HOME/.asdf/installs/mage/1.13.0/bin/mage -debug -l" Benchmark 1: /Users/jaredallard/.asdf/installs/mage/1.13.0/bin/mage -debug -l Time (mean ± σ): 529.2 ms ± 12.7 ms [User: 196.9 ms, System: 433.4 ms] Range (min … max): 507.8 ms … 555.5 ms 10 runs