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(codegen): create go.mod file automatically #405

Merged
merged 17 commits into from
Jan 25, 2023

Conversation

eduardomourar
Copy link
Contributor

@eduardomourar eduardomourar commented Dec 24, 2022

Issue #, if available: N/A

Description of changes:

In order to simplify the management of the generated Go files and well as allowing some customization, this will introduce the following changes:

  • A property called moduleVersion in the codegen setttings will allow consumers to pass the version that they manage user their own tooling outside Smithy
  • The file go.mod will automatically be generated whenever a settings called generateGoMod is set to true based on the template located at codegen/smithy-go-codegen/src/main/resources/software/amazon/smithy/go/codegen/go.mod.template
  • The file go_module_metadata.go will generated containing the version of the module provided by the user

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@eduardomourar eduardomourar requested a review from a team as a code owner December 24, 2022 01:19
@syall
Copy link
Contributor

syall commented Dec 28, 2022

Hi @eduardomourar, thanks for the contribution!

I was wondering if it would be possible to provide an example of how this feature would work, and how this change was tested?

@eduardomourar
Copy link
Contributor Author

At Stedi, we are starting to generate the Go client SDKs based on our own Smithy models. In order to use our tooling for managing the modules' version, it made more sense to create, at least, the go.mod as part of the codegen. We have tested this internally with all services, but we are not ready to make this repository public just yet. I will try to either create a sample repo or maybe include some basic test to this PR.

@RanVaknin
Copy link
Contributor

RanVaknin commented Dec 29, 2022

Hi @eduardomourar

We really appreciate your contribution! Right now no one is able to review your PR. We will try to address is as part of 2023 efforts.

Thanks again!
Ran

@syall syall self-assigned this Jan 12, 2023
@syall
Copy link
Contributor

syall commented Jan 18, 2023

Hey @eduardomourar! I had a few suggestions for this feature:

  1. Toggle go.mod generation through a generateGoMod boolean in GoSettings.java (defaulting to false) to avoid overwriting existing consumers that have their own go.mod workflows.
  2. Use go commands to generate go.mod files (reference Generated Manifest of Build Artifacts #283, where go.mod generation was first removed from smithy-go).
  3. For go.mod, use a goDirective string in GoSettings.java (defaulting to 1.15) instead of DEFAULT_MINIMUM_GO_VERSION in the PR (see the go module docs for reference).
  4. For the go_module_metadata.go generation, this is an opinionated file that probably should not be included in the smithy-go repo. If it were included, it should not be in the smithy-go plugins SPI list, but instead used at a smithy-go consumer level (e.g. aws-sdk-go-v2).

@syall syall self-requested a review January 18, 2023 17:33
@syall
Copy link
Contributor

syall commented Jan 18, 2023

The go.mod unit test probably failed since go.mod isn't written into the smithy file manifest, but is instead written by go commands.

To assert on go.mod contents and creation, the actual path of go.mod would have to be referenced.

@eduardomourar
Copy link
Contributor Author

The go.mod unit test probably failed since go.mod isn't written into the smithy file manifest, but is instead written by go commands.

To assert on go.mod contents and creation, the actual path of go.mod would have to be referenced.

I tried but it is actually failing while running the go command with read-only file system

Copy link
Contributor

@syall syall left a comment

Choose a reason for hiding this comment

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

Left a few comments, but overall looks good.


The go.mod output in smithy-go-codegen-test gradle subproject looks correct at path codegen/smithy-go-codegen-test/build/smithyprojections/smithy-go-codegen-test/source/go-codegen/go.mod:

module github.com/aws/smithy-go/internal/tests/service/weather

go 1.18

require (
        github.com/aws/smithy-go v1.4.0
        github.com/jmespath/go-jmespath v0.4.0
)

Co-authored-by: Steven Yuan <s.yuan.all@gmail.com>
@eduardomourar eduardomourar requested a review from a team as a code owner January 24, 2023 13:57
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.

None yet

4 participants