Skip to content

Conversation

@rata
Copy link
Member

@rata rata commented Apr 28, 2025

To not accidentally break go install, let's add CI to check it. If in the future we need those directives, we can remove the CI check.

This should only be merged after #4748, as we are currently using an exclude directive.

@rata rata force-pushed the go-mod-exclude-linter branch 2 times, most recently from ab9162c to 182ddeb Compare April 28, 2025 10:42
@rata
Copy link
Member Author

rata commented Apr 28, 2025

It fails as expected now :)

@rata rata force-pushed the go-mod-exclude-linter branch from 182ddeb to e06dc93 Compare April 29, 2025 11:46
@rata
Copy link
Member Author

rata commented Apr 29, 2025

Rebased now that is merged

To not accidentally break `go install`, let's add CI to check it. If in
the future we need those directives, we can remove the CI check.

Signed-off-by: Rodrigo Campos <[email protected]>
@rata rata force-pushed the go-mod-exclude-linter branch from e06dc93 to 9f86496 Compare April 30, 2025 15:30
@rata
Copy link
Member Author

rata commented Apr 30, 2025

@AkihiroSuda PTAL

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

lgtm

@AkihiroSuda AkihiroSuda merged commit 8d90e3d into opencontainers:main Apr 30, 2025
34 checks passed
@kolyshkin
Copy link
Contributor

kolyshkin commented May 1, 2025

One other way to do that is to remove exclude/replace when opening release branches. For example, this is what gopls does: golang/go#42627 / https://go-review.googlesource.com/c/tools/+/272688. In their master branch, there's a replace, and it gets removed once a release branch is created (starting from gopls/v0.5.0-pre.1 or so).

We don't have to do the same thing here because we don't have multiple inter-dependent go.mod files in the same repository, but moby/sys might actually benefit from this trick (this will result in more git noise and complicated releases but simpler CI). Cc @thaJeztah

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.

4 participants