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(app_v2): detect app wiring implementation #3434

Merged
merged 12 commits into from
Mar 29, 2023
Merged

Conversation

Pantani
Copy link
Collaborator

@Pantani Pantani commented Feb 13, 2023

Headline

Create a method to detect if the chain is using the app wiring trying to find the depinject.Inject declaration into the app package files

@Pantani Pantani self-assigned this Feb 13, 2023
@Pantani Pantani added the skip-changelog Don't check changelog for new entries label Feb 13, 2023
Copy link
Member

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

I think checking against depinject.Inject (which should always present in v2 of app.go) might be a better option instead of Compose. Because compose is only used if you are defining app configuration through a Go file, alternatively you can use the app.yml format and load it in Go but this way Compose'ing will be done in the yaml file, not by calling a Go function.

@Pantani Pantani marked this pull request as draft February 20, 2023 07:34
@Pantani Pantani marked this pull request as ready for review February 22, 2023 23:22
@Pantani Pantani removed the skip-changelog Don't check changelog for new entries label Feb 24, 2023
@Pantani Pantani requested a review from ilgooz February 24, 2023 07:00
@Pantani Pantani requested a review from ilgooz March 23, 2023 02:05
@ilgooz
Copy link
Member

ilgooz commented Mar 29, 2023

@jeronimoalbi can you please review this one?

jeronimoalbi

This comment was marked as duplicate.

@jeronimoalbi
Copy link
Member

There are a couple of network test failing but otherwise LGTM 👍

@jeronimoalbi
Copy link
Member

I think the reason tests fail is because SPN app format is V1, so there might be an issue in the detection process.

@ilgooz ilgooz merged commit a34ee06 into main Mar 29, 2023
@ilgooz ilgooz deleted the feat/detect-app-wiring branch March 29, 2023 16:05
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* detect cosmos chain app wiring

* check if the appconfig import exist

* fix lint method comment

* draft some changes to improve the var assign search with ast

* improve the declaration search and find the app wiring implementation finding the `depinject.Inject` declaration

* add changelog

* add test data to a file

* change the appImplementation method names to find the app.go

* fix app.go search

---------

Co-authored-by: İlker G. Öztürk <[email protected]>
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

3 participants