Skip to content

[go] clean up gen of go types #15109

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

Merged
merged 1 commit into from
Dec 1, 2022
Merged

[go] clean up gen of go types #15109

merged 1 commit into from
Dec 1, 2022

Conversation

akosyakov
Copy link
Member

Description

In context of #15062 we need to pass config from server to ide-service.
But because of current code generator we cannot unmarshall it and marshall back additional tasks.
This PR simplifies code generated output that it has only types and rely on default go logic for marshalling.

Related Issue(s)

Related to #9189

How to test

Start a workspace with IntelliJ and VS Code with .gitpod.yml with extensions/plugins/ports and see whether UI respect it.

Release Notes

NONE

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

In order to allow unmarshalling of config passed from server to ide-service and marhsalling back as JSON.
@akosyakov akosyakov requested a review from a team December 1, 2022 14:26
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-ak-go-prot-gen.1 because the annotations in the pull request description changed
(with .werft/ from main)

@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Dec 1, 2022
@akosyakov akosyakov requested a review from a team December 1, 2022 14:27
@akosyakov
Copy link
Member Author

akosyakov commented Dec 1, 2022

@geropl webapp team is assigned, but only IDE team is using it 😄 I asked @iQQBot to test

/hold

@iQQBot feel free to unhold if IDE still can parse yaml

@easyCZ
Copy link
Member

easyCZ commented Dec 1, 2022

To what extend do we control the gitpod-config-types.go generated code? Could we adjust the generator to generate more appropriate code for us? I don't have as much context on this so you may have already explored this.

@akosyakov
Copy link
Member Author

@easyCZ We use github.com/a-h/generate. We could fork it, but I think this PR already does what we need.

@iQQBot
Copy link
Contributor

iQQBot commented Dec 1, 2022

To what extend do we control the gitpod-config-types.go generated code? Could we adjust the generator to generate more appropriate code for us? I don't have as much context on this so you may have already explored this.

maybe it's a good idea, we can fork this repo and do some change. cc @akosyakov

@akosyakov
Copy link
Member Author

@iQQBot ok, it is up to you

@easyCZ
Copy link
Member

easyCZ commented Dec 1, 2022

/hold to approve given you're the appropriate owners (and don't want to block you) but in case you want to make some changes

@akosyakov
Copy link
Member Author

akosyakov commented Dec 1, 2022

/werft run recreate-vm=true

👍 started the job as gitpod-build-ak-go-prot-gen.2
(with .werft/ from main)

Copy link
Contributor

@iQQBot iQQBot left a comment

Choose a reason for hiding this comment

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

Test with gitpod repo, it works

@iQQBot
Copy link
Contributor

iQQBot commented Dec 1, 2022

/unhold

we discuss we use this way first, and do a fork if we still need some advanced change

@roboquat roboquat merged commit 8374099 into main Dec 1, 2022
@roboquat roboquat deleted the ak/go_prot_gen branch December 1, 2022 16:18
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed: IDE IDE change is running in production deployed Change is completely running in production labels Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/XXL team: IDE team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants