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

Replace common.sh with Rust code #358

Merged
merged 3 commits into from Feb 27, 2019

Conversation

Projects
None yet
3 participants
@ice1000
Copy link
Member

commented Feb 24, 2019

Signed-off-by: ice1000 ice1000kotlin@foxmail.com

So we have fewer files in the root directory in this repo and no longer depend on sed.

Replace common.sh with Rust code
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>

@ice1000 ice1000 added the enhancement label Feb 24, 2019

@ice1000 ice1000 requested a review from nrc Feb 24, 2019

@ice1000

This comment has been minimized.

Copy link
Member Author

commented Feb 24, 2019

Ooops, I didn't know generate_go.sh depends on common.sh too...

@ice1000

This comment has been minimized.

Copy link
Member Author

commented Feb 24, 2019

I'm fine with closing this but I'd like to listen to others' opinions.

@nrc

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2019

seems like a win to me?

@Hoverbear

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

This should fix #355

panic!(
"Invalid version of protoc (required 3.1.x), or protoc not installed\n\nstdout:\n\n{}",
String::from_utf8_lossy(&output.stdout)
"Invalid version of protoc (required 3.1.x, get {}.{}.x).",

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Feb 25, 2019

Member
Suggested change
"Invalid version of protoc (required 3.1.x, get {}.{}.x).",
"Invalid version of protoc (required 3.1.x, got {}.{}.x).",
@Hoverbear
Copy link
Member

left a comment

Please leave common.sh so the Go build can pass. We can fix #355 with this.

If you'd prefer, could you fix the functionality for the Go build? :)

The rest LGTM.

Add common.sh's content back
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>

@ice1000 ice1000 force-pushed the ice1000:no-common.sh branch from 726def5 to 657f5d0 Feb 25, 2019

@Hoverbear
Copy link
Member

left a comment

Rest LGTM

@ice1000

This comment has been minimized.

Copy link
Member Author

commented Feb 26, 2019

PTAL @nrc

@nrc

nrc approved these changes Feb 26, 2019

Copy link
Contributor

left a comment

lgtm!

@ice1000 ice1000 merged commit e71ca01 into pingcap:master Feb 27, 2019

3 checks passed

ci/circleci: go Your tests passed on CircleCI!
Details
ci/circleci: rust Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.