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

Go: Ignore Go workspace file go.work #3884

Merged
merged 1 commit into from
Dec 4, 2021
Merged

Conversation

breml
Copy link
Contributor

@breml breml commented Nov 13, 2021

Reasons for making this change:

With Go 1.18, support for Go workspaces will land. Go workspaces are configured in go.work, which contains paths to local development versions of modules and therefore is not expected to be commited.

Links to documentation supporting these rule changes:

With Go 1.18, support for Go workspaces will land.
Go workspaces are configured in `go.work`, which
contains paths to local development versions of
modules and therefore is not expected to be
commited.

See:
* golang/go#45713
@bdougie
Copy link
Contributor

bdougie commented Dec 4, 2021

Thanks for linking the documentation. I am referencing this line from Sebastian's article:

In usual project situations, it’s recommended to not commit the go.work file, as its main use case is local development.

@goto1134
Copy link

Hey, @breml! Should we also add go.work.sum files to the list?

@breml
Copy link
Contributor Author

breml commented Aug 21, 2023

@goto1134 I did not look into the details, but I guess adding go.work.sum to the .gitignore file for Go would make sense if go.work is excluded already.

@goto1134
Copy link

Oh, I believe #4081 is about that.

@alvaroaleman
Copy link

This is a terrible change. It means that any form of cross-module IDE feature doesn't work by default anymore for gopls users. Why would a multi-module repository want IDE features to be broken by default?

@goto1134
Copy link

@alvaroaleman. Consider working in a large mono repo. Different teams might need to have different workspace configurations for their local development. I believe the general idea about excluding the file from Git is not that bad.

On the other hand, it is not that common for people to work in mono repo. Therefore, the default behavior should not restrict go.work. We could comment these lines, as it was done in #3033.

@alvaroaleman
Copy link

@alvaroaleman. Consider working in a large mono repo. Different teams might need to have different workspace configurations for their local development. I believe the general idea about excluding the file from Git is not that bad.

Any concrete example for this? Why would different teams want different workspace configurations within the same repo? Very typical examples of using multiple modules within the same repo include:

  • A distinct api submodule
  • A distinct client submodule

Not having a go.work means that LSP won't work across the submodule borders. This in turn means that you can for example can not do things like "Find references for this field in my api module" which is tremendously annoying.

@goto1134
Copy link

goto1134 commented Mar 27, 2024

The original proposal for the feature says:

go.work files should not be checked into version control repos containing modules so that the go.work file in a module does not end up overriding the configuration a user created themselves outside of the module. The go.work documentation should contain clear warnings about this.
https://go.googlesource.com/proposal/+/master/design/45713-workspace.md#rationale-the-file

The exact problematic scenarios are discussed in the issue golang/go#53502. In the ticket Michael Matloob (the author of the proposal) mentions the disadvantages of checking-in the files.

@alvaroaleman
Copy link

He also mentions reasons why it makes sense, so blanket assuming one seems wrong.

@goto1134
Copy link

goto1134 commented Mar 28, 2024

On the other hand, it is not that common for people to work in mono repo. Therefore, the default behavior should not restrict go.work. We could comment these lines, as it was done in #3033.

Do you think, it could be a solution to your problem?

@chehsunliu
Copy link

It's so weird to accept this change... You wouldn't put package.json into .gitignore just because it can point to a local path.

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.

7 participants