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

task: update documentation for golang #3338

Merged
merged 1 commit into from
Oct 23, 2024
Merged

task: update documentation for golang #3338

merged 1 commit into from
Oct 23, 2024

Conversation

wtrocki
Copy link
Member

@wtrocki wtrocki commented Oct 22, 2024

Description

Updating documentation to cover golang version requirement

Reason

 Building atlas binary
go build -ldflags "-s -w -X github.com/mongodb/mongodb-atlas-cli/atlascli/internal/version.GitCommit=436cdc259407d225f622e5a407a43e43f4070d99 -X github.com/mongodb/mongodb-atlas-cli/atlascli/internal/version.Version=1.19.0-374-g436cdc259"  -o ./bin/atlas ./cmd/atlas
go: go.mod requires go >= 1.23.1 (running go 1.21.3; GOTOOLCHAIN=local)
make: *** [build] Error 1

Separate concern

Go.mod file does not use toolchain directive which can automatically request proper golang version for build purposes.
Do we have any reason to not define tooling directive?
Since I moved from Atlas CLI dev I noticed we no longer use adsf tools definitions.

## Description

Updating documentation to cover golang version requirement

## Reason
```
 Building atlas binary
go build -ldflags "-s -w -X github.com/mongodb/mongodb-atlas-cli/atlascli/internal/version.GitCommit=436cdc259407d225f622e5a407a43e43f4070d99 -X github.com/mongodb/mongodb-atlas-cli/atlascli/internal/version.Version=1.19.0-374-g436cdc259"  -o ./bin/atlas ./cmd/atlas
go: go.mod requires go >= 1.23.1 (running go 1.21.3; GOTOOLCHAIN=local)
make: *** [build] Error 1
```
## Separate concern

Go.mod file does not use toolchain directive which can automatically request proper golang version to be used. 

Since I moved from Atlas CLI I noticed we no longer use adsf tools definitions.
@wtrocki wtrocki requested a review from a team as a code owner October 22, 2024 11:12
@wtrocki
Copy link
Member Author

wtrocki commented Oct 23, 2024

Thank you, @blva Created #3340 as follows up to enable toolchain support.

@wtrocki wtrocki merged commit 5664451 into master Oct 23, 2024
21 checks passed
@wtrocki wtrocki deleted the task-go-doc branch October 23, 2024 11:32
@gssbzn
Copy link
Collaborator

gssbzn commented Oct 29, 2024

Do we have any reason to not define tooling directive?

https://github.com/mongodb/mongodb-atlas-cli/pull/2779/files#r1530657881

@wtrocki
Copy link
Member Author

wtrocki commented Oct 29, 2024

Thank you for reference. I seen that PR and I did also removed adsf support for the repository:
https://github.com/mongodb/mongodb-atlas-cli/pull/2779/files#r1530658440

My understanding is that we want autoinstallation feature to be disabled.
That would mean this PR was correct (bumped required version that needs to be installed manually).
Closing toolchain PR that adds extra auto option as fallback to local

@gssbzn
Copy link
Collaborator

gssbzn commented Oct 29, 2024

At the time I think we discussed offline (sorry we didn't capture it in the PR) given using both asdf and auto toolchain is an issue

  • do we disable auto toolchain and prefer asdf
  • do we disable asdf and prefer toolchain

And we decided to disable both and let devs decide what to use, in particular toolchain is disabled for the makefile because installation of the dev tools is wonky with the autotool detection

@wtrocki
Copy link
Member Author

wtrocki commented Oct 29, 2024

@gssbzn Thank you. All makes sense. I tried to look if we can add any information into contributing guide but we already mention manual installation for specific version and do not mention toolchain/asdf support anywhere. All good.

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