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

refactor: cleanup tmp folders & change third party includes #3696

Merged
merged 15 commits into from
Oct 17, 2023

Conversation

jeronimoalbi
Copy link
Member

@jeronimoalbi jeronimoalbi commented Oct 16, 2023

No description provided.

Use `thirdModuleIncludes` instead of defining a new type to store
modules and includes to be consistent with `appIncludes` and to avoid
changing existing code to work with a new type.
@jeronimoalbi
Copy link
Member Author

Applies to #3655

@jeronimoalbi jeronimoalbi added the skip-changelog Don't check changelog for new entries label Oct 16, 2023
Currently CI is giving an error because is not properly configured to
support the new Go 1.21 features for the mod file and we don't have a
consensus yet on how to configure the toolchain and Go versions.
@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

Merging #3696 (c8a21e1) into chore/revert-ts-codegen (e32b646) will decrease coverage by 0.02%.
The diff coverage is 9.09%.

Additional details and impacted files

Impacted file tree graph

@@                     Coverage Diff                     @@
##           chore/revert-ts-codegen    #3696      +/-   ##
===========================================================
- Coverage                    24.71%   24.69%   -0.02%     
===========================================================
  Files                          289      289              
  Lines                        23925    23943      +18     
===========================================================
  Hits                          5912     5912              
- Misses                       17482    17500      +18     
  Partials                       531      531              
Files Coverage Δ
ignite/pkg/cache/cache.go 65.11% <0.00%> (ø)
ignite/pkg/cosmosanalysis/app/app.go 65.87% <80.00%> (ø)
ignite/pkg/cosmosbuf/buf.go 4.51% <0.00%> (ø)
ignite/pkg/cosmosgen/generate_composables.go 0.00% <0.00%> (ø)
ignite/pkg/cosmosgen/generate_openapi.go 0.00% <0.00%> (ø)
ignite/pkg/cosmosgen/generate_vuex.go 0.00% <0.00%> (ø)
ignite/pkg/placeholder/error.go 8.88% <50.00%> (ø)
ignite/pkg/nodetime/programs/ts-proto/tsproto.go 0.00% <0.00%> (ø)
ignite/pkg/cosmosgen/generate_typescript.go 0.00% <0.00%> (ø)
ignite/pkg/cosmosgen/cosmosgen.go 3.33% <0.00%> (-0.24%) ⬇️
... and 1 more

This is required to avoid downloading a newer Go toolchain which will
fail because GOSUMDB is disabled to fix the timeout issues because of
the repository size.
This is required because toolchain won't work when GOSUMDB is disabled.
The stable Go version should be used to avoid keeping updating the
workflow config when a new patch version is released. This is to make
sure that Go doesn't try to download a new toolchain version.
Copy link

@Cplus360 Cplus360 left a comment

Choose a reason for hiding this comment

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

Reviewed

Go setup action caches since `v4`
Action for `golangci-lint` already have caching functionality so Go setup
must be disabled to avoid caching issues.
@jeronimoalbi jeronimoalbi marked this pull request as ready for review October 16, 2023 17:27
Copy link
Collaborator

@Pantani Pantani left a comment

Choose a reason for hiding this comment

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

can you please add more description about what this PR does?

ignite/pkg/cosmosgen/generate.go Outdated Show resolved Hide resolved
@jeronimoalbi
Copy link
Member Author

can you please add more description about what this PR does?

Yes, sorry, fixes/refactor got more complex than I though originally.

  1. Fix CI lint and integration tests workflow which stop working when the go.mod was updated. The version was actually updated by Go because nodetime dependency uses Go release v1.21.1 which means that the CLI must also use v1.21.1. That introduced some issues with golangci-lint and Go toolchain that didn't work because it needs GOSUMDB enabled.

  2. Fix linting issues.

  3. Remove temporary folders generated when exporting proto files using Buf, otherwise they won't be removed after code generation.

  4. Introduce thirdModuleIncludes in pkg/cosmosgen generator to be consistent with the existing implementation which uses appModules and appIncludes, to remove a new struct type, to be more semantic and to avoid refactoring existing code that should change if a new struct is introduced instead of the thirdModuleIncludes

@jeronimoalbi jeronimoalbi merged commit 19c2666 into chore/revert-ts-codegen Oct 17, 2023
26 checks passed
@jeronimoalbi jeronimoalbi deleted the chore/revert-ts-codegen-fixes branch October 17, 2023 12:46
Pantani added a commit that referenced this pull request Oct 24, 2023
* feat: Re-introduce module discovery

* feat: Look for indirect proto deps in go.mod

* feat: Introduce indirect proto dependency discovery

* feat: Add buf export command

* feat: User buf-based openapi generation

* fix: Revert to previous sta version & peg TS version

* feat(wip): Update dependency resolution algorithm

* feat: Clean up include resolution

* chore: Add changelog entry

* chore: Fix linting issues & typos

* fix: cosmos-sdk buf issue

* feat(pkg/protoc): change package to use protoc binary from files repo (#3657)

* feat(pkg/protoc): change package to use protoc binary from files repo

* chore(pkg/protoc): remove embedded protoc binary

* fix: correct buf.gen.sta.yml file

* fix: address review

* fix: Remove logging

* fix: ResolveDependencies call

* fix: correct lint issues and merge `main` (#3690)

* chore: remove redundant variable

* chore(pkg/cosmosbuf): improve snippet redability

* chore: remove redundant default value

* chore: code formatting

* feat(pkg): remove nodetime binaries (#3670)

* - remote nodetime binaries
- import ignite-files repo pkg
- remote gen binaries scripts and pipeline
- run gofmt

* remove tollchain from go.mod

* add changelog

* Update changelog

* ci: update Go version to `1.21` in GitHub workflows (#3687)

---------

Co-authored-by: Pantani <Pantani>
Co-authored-by: Jerónimo Albi <[email protected]>

* fix: change Go version in nightly release config to `1.21.2` (#3688)

* fix: change Go version in nightly release config to `1.21.2`

The release action requires a version that includes the patch version to
be able to download the right tarball with the Go binary.

See https://github.com/wangyoucao577/go-release-action/blob/2ac3035fa4c4feed6a8272ce278b0577b93cf8e5/setup-go.sh#L24

* chore: change go releaser to use the lates Go version

Latest is the default but it's setted here to be explicit about that fact.
See https://github.com/wangyoucao577/go-release-action#parameters

* fix: remove explicit "latest" for Go version release nightly (#3689)

Lates version is used by default but it shouldn't be specified
explicitly because the value "latest" is not supported.

* chore: merge `main`

* test: fix broken integration test

* fix: change scaffolder to skip protoc when no module is scaffolded

There are no proto file when an app is scaffolded without module which
makes buf export fail because buf workspace references a folder without
proto files.

* fix(pkg/cosmosbuf): add check for proto files before running Buf

Changes previous commit because some files like the OpenAPI file should
be generated even when there are no proto files. We should also allow
generating code for standard dependencies like Comos SDK when the app
doesn't have proto files.

---------

Co-authored-by: Danilo Pantani <[email protected]>

* feat: Bump nodetime version

* refactor: cleanup tmp folders & change third party includes (#3696)

* fix: implement third party includes using previous semantics

Use `thirdModuleIncludes` instead of defining a new type to store
modules and includes to be consistent with `appIncludes` and to avoid
changing existing code to work with a new type.

* feat: add tmp dir cleanup support to cosmos generator

* chore: restore Go version to `1.21` and remove toolchain

Currently CI is giving an error because is not properly configured to
support the new Go 1.21 features for the mod file and we don't have a
consensus yet on how to configure the toolchain and Go versions.

* chore: go mod tidy

* ci: change integration tests to use Go version from mod file

This is required to avoid downloading a newer Go toolchain which will
fail because GOSUMDB is disabled to fix the timeout issues because of
the repository size.

* ci: disable GOTOOLCHAIN when running integration tests

This is required because toolchain won't work when GOSUMDB is disabled.

* ci: change integration tests to use the stable go version

The stable Go version should be used to avoid keeping updating the
workflow config when a new patch version is released. This is to make
sure that Go doesn't try to download a new toolchain version.

* fix: correct CI linting issues

* ci: remove redundant GH workflow cache step

Go setup action caches since `v4`

* fix: disable go setup GH action cache

Action for `golangci-lint` already have caching functionality so Go setup
must be disabled to avoid caching issues.

* chore: add thitd party includes only when available

* fix: correct directory remove

Co-authored-by: Danilo Pantani <[email protected]>

* fix: Address code review comments

* chore: Address review comment

* chore: remove commented function

* Update ignite/pkg/cosmosbuf/buf.go

Co-authored-by: Jerónimo Albi <[email protected]>

* chore: fix function name

* Update ignite/pkg/cosmosgen/generate_openapi.go

Co-authored-by: Danilo Pantani <[email protected]>

* chore: address review

* chore: linting

* chore: test --experimental_allow_proto3_optional flag

---------

Co-authored-by: Jerónimo Albi <[email protected]>
Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: jeronimoalbi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:ci CI/CD workflow and automated jobs. component:configs component:packages skip-changelog Don't check changelog for new entries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants