search

apache / incubator-pegasus

Apache Pegasus - A horizontally scalable, strongly consistent and high-performance key-value store
https://pegasus.apache.org/
Apache License 2.0
1.99k stars 312 forks source link

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

Closed lengyuexuexuan closed 5 months ago

lengyuexuexuan commented 9 months ago

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
acelyc111 commented 8 months ago

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

acelyc111 commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

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 commented 5 months ago

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

lengyuexuexuan commented 5 months ago

@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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

@lengyuexuexuan please check this issue as well

https://github.com/apache/incubator-pegasus/actions/runs/9285106961/job/25848921042?pr=1917#step:6:4621

lengyuexuexuan commented 5 months ago

@lengyuexuexuan please check this issue as well

https://github.com/apache/incubator-pegasus/actions/runs/9285106961/job/25848921042?pr=1917#step:6:4621

done.