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

feat: use go.work to reduce core library dependencies #257

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

slsyy
Copy link
Contributor

@slsyy slsyy commented Feb 2, 2025

some shenaningas were introduced to run tests&lint over all modules Probably it won't be necessary, when
golang/go#71294 is resolved

Comment on lines 35 to +43
- uses: actions/checkout@v4

- name: golangci-lint
uses: golangci/golangci-lint-action@v4
- uses: actions/setup-go@v5
with:
go-version: ${{ env.GO_VERSION }}
- name: golangci-lint ${{ matrix.modules }}
uses: golangci/golangci-lint-action@v6
with:
version: v1.56.2
version: ${{ env.GOLANGCI_LINT_VERSION }}
working-directory: ${{ matrix.modules }}

Choose a reason for hiding this comment

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

I'm unsure to understand why you are computing all modules then run sequentially via the matrix

For me, as long as you have a go.work, go test ./... and golangci-lint run ./... should work in a transverse way

So for me, you don't need the go list -json | jq

Could you break something dbtests/mysql_test.go either with code and style

Then try to run go test and golangci-lint

They should be detected.

It's worth trying

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me, as long as you have a go.work, go test ./... and golangci-lint run ./... should work in a transverse way

Unfortunately it does not work in that way, you need to run go test in each go.mod directory, check this golang/go#71294

Anyway I am still experimenting and I will try few different ideas to check, which one least sucks

ty for your input

Choose a reason for hiding this comment

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

For anyone interested

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.

Originally posted by @rsc in golang/go#71294 (comment)

🤞

Copy link

@ccoVeille ccoVeille Feb 6, 2025

Choose a reason for hiding this comment

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

Unfortunately it does not work in that way, you need to run go test in each go.mod directory, check this golang/go#71294\n\nAnyway I am still experimenting and I will try few different ideas to check, which one least sucks

Hi @slsyy

While I'm now convinced about the lack of feature in go test, I'm doubtful about the fact golangci-lint doesn't support it

A few weeks ago, I added a simple go.work file in gofr framework

And all the errors appeared locally when I was running golangci-lint manually locally

@slsyy slsyy marked this pull request as draft February 2, 2025 17:18
some shenaningas were introduced to run tests&lint over all modules
Probably it won't be necessary, when
golang/go#71294 is resolved
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.

2 participants