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 go workspaces #1784

Closed
wants to merge 4 commits into from
Closed

use go workspaces #1784

wants to merge 4 commits into from

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Jul 26, 2022

Description

The go workspaces feature added to go in v1.18 allows for easier management of multi-module repos, here's some discussion from the sdk:

Think of go.work.sum as kinda-sorta "the state of the whole repo including all modules in it"


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@faddat faddat changed the title use go workspaces so editors don't get crazy use go workspaces Jul 26, 2022
@faddat faddat mentioned this pull request Jul 28, 2022
9 tasks
@colin-axner
Copy link
Contributor

Hi @faddat, thanks for opening the pr! In general I'm in favor of the addition, though I have two concerns from my very limited understanding of workspaces (just quickly reading up on it now)

I share the concerns expressed on the SDK pr around removing go.work and go.work.sum for releases as it seems it could cause an unexpected issues? Was there a conclusion regarding this?

How will workspaces work when you explicitly want one module of this repo to use an older version of another module in this repo. Lets say in the future, each light client or application has its own go.mod and we want to make changes to core IBC, release that change and then apply the changes to the apps/light clients later. My understanding is that go.work would force these separate modules to use the current version of core IBC. How would this be turned off? Maybe I am missing something?

@faddat
Copy link
Contributor Author

faddat commented Jul 28, 2022

Apologies, I don't yet know.

Benefit: less issues on the editor side

Downside: I think we are all a bit in the dark here as to how exactly this plays out over months or years

@crodriguezvega
Copy link
Contributor

crodriguezvega commented Aug 7, 2022

Should we then close this PR for now and revisit later on when we have more clarity about the consequences of such change?

@colin-axner
Copy link
Contributor

That sounds good to me. I think you can add it to your .gitignore if you'd like to use it during development

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants