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

build(golang): Support faster local iteration and align better with upstream #8840

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

gibson042
Copy link
Member

Description

  • Replace synthetic "go-mod-cache" Makefile target with direct dependence upon go.sum (as upstream).
  • Introduce support for make SKIP_MOD_VERIFY=1 to avoid unnecessary expensive scans when e.g. tweaking referents of local-directory replace $modulePath => ../path/to/module directives in go.mod.

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

There is no formal place to document the new make variable, so it is instead directly mentioned in make output.

Testing Considerations

Manually verified expected behavior in various scenarios (e.g., running make without go.mod changes).

Upgrade Considerations

n/a

Copy link
Contributor

@JimLarson JimLarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo comments below.

Comment on lines -80 to -82
go-mod-cache: go.sum
@echo "--> Download go modules to local cache"
@go mod download
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep go-mod-cache in the Makefile, do avoid differences vs gaia.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, can you elaborate? AFAICT, cosmos-sdk hasn't had a "go-mod-cache" target since it was removed by cosmos/cosmos-sdk@6c1c2cc .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened a PR there: cosmos/gaia#2915

golang/cosmos/Makefile Show resolved Hide resolved
@gibson042 gibson042 added the automerge:rebase Automatically rebase updates, then merge label Aug 29, 2024
@gibson042 gibson042 force-pushed the gibson-2024-01-faster-make-agd branch from dbba9ce to e2ff3af Compare August 29, 2024 15:23
Copy link

cloudflare-workers-and-pages bot commented Aug 29, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 670cb8a
Status: ✅  Deploy successful!
Preview URL: https://5e341b7b.agoric-sdk.pages.dev
Branch Preview URL: https://gibson-2024-01-faster-make-a.agoric-sdk.pages.dev

View logs

@gibson042 gibson042 force-pushed the gibson-2024-01-faster-make-agd branch from 88bd504 to 0d09636 Compare August 29, 2024 17:48
…pstream

* Replace synthetic "go-mod-cache" Makefile target with direct dependence
  upon go.sum.
* Introduce support for `make SKIP_MOD_VERIFY=1` to avoid unnecessary
  expensive scans when e.g. tweaking referents of local-directory
  `replace $modulePath => ../path/to/module` directives in go.mod.
@gibson042 gibson042 force-pushed the gibson-2024-01-faster-make-agd branch from 0d09636 to 670cb8a Compare August 29, 2024 18:21
Copy link
Contributor

@JeancarloBarrios JeancarloBarrios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mergify mergify bot merged commit bd64eda into master Aug 29, 2024
80 checks passed
@mergify mergify bot deleted the gibson-2024-01-faster-make-agd branch August 29, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants