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

chore(go-client): add generation thrift files of go-client #1917

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lengyuexuexuan
Copy link
Collaborator

What problem does this PR solve?

#1881

What is changed and how does it work?

By uploading generation thrift files, the go client can be used directly by users through "go get" without the need to compile it locally.

Tests
  • Manual test (add detailed scripts or steps below)

@acelyc111
Copy link
Member

@lengyuexuexuan Please check the failed CIs, for Check License, you need to add the new files to .licenserc.yaml.

@acelyc111
Copy link
Member

Now that the go files have been pre-generated, the generate steps in https://github.com/apache/incubator-pegasus/blob/master/go-client/Makefile#L19 can be removed, right? Then you need to add the manual generate steps in go-client/README.md.

And more, you need to add steps in .github/workflows/lint_and_test_go-client.yml to check that the pre-generated go files are the same to the ones generated manually.

@lengyuexuexuan
Copy link
Collaborator Author

Now that the go files have been pre-generated, the generate steps in https://github.com/apache/incubator-pegasus/blob/master/go-client/Makefile#L19 can be removed, right? Then you need to add the manual generate steps in go-client/README.md.

And more, you need to add steps in .github/workflows/lint_and_test_go-client.yml to check that the pre-generated go files are the same to the ones generated manually.

Is it possible to remove the thrift file directly in the .gitignore file, so that if the thrift file is modified, it can be uploaded directly?

@acelyc111
Copy link
Member

Now that the go files have been pre-generated, the generate steps in https://github.com/apache/incubator-pegasus/blob/master/go-client/Makefile#L19 can be removed, right? Then you need to add the manual generate steps in go-client/README.md.
And more, you need to add steps in .github/workflows/lint_and_test_go-client.yml to check that the pre-generated go files are the same to the ones generated manually.

Is it possible to remove the thrift file directly in the .gitignore file, so that if the thrift file is modified, it can be uploaded directly?

Of course, feel free to do that.

acelyc111
acelyc111 previously approved these changes May 23, 2024
@acelyc111
Copy link
Member

@lengyuexuexuan Do you have some time to improve the PR ? I'm pleased to give you some help if needed.

@lengyuexuexuan
Copy link
Collaborator Author

@lengyuexuexuan Do you have some time to improve the PR ? I'm pleased to give you some help if needed.

thanks. Today or Tomorrow I will improve this pr.

@lengyuexuexuan
Copy link
Collaborator Author

@acelyc111
Now I have uploaded the latest version of the thrift file, and remove go-client/idl/* in .gitignored file.
I tested it and now users can directly pull the latest version of go client and use it directly.
It needs another committer to aprrove this pr and #1916.

@acelyc111
Copy link
Member

according to the error, you need to add these file names [1] to .licenserc.yaml (add them in property place to keep in dictionary order).

  1. https://github.com/apache/incubator-pegasus/actions/runs/9285106967/job/25548898496?pr=1917#step:3:141

@acelyc111
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants