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

Support no gopackage #101

Merged
merged 6 commits into from
Feb 15, 2022
Merged

Support no gopackage #101

merged 6 commits into from
Feb 15, 2022

Conversation

jekiapp
Copy link
Contributor

@jekiapp jekiapp commented Feb 2, 2022

Motivation

The current version of gripmock is requiring a declaration of option go_package. Thus I expect many issues since the user of gripmock is not always a Golang user.

Technically, these changes are not making gripmock able to run without option go_package, but it will generate the declaration of it based on the .proto files provided. Since it only uses a simple bash script to do that, I will release this version as a beta release before making a stable release.

Changelist

fix_gopackage.sh is the core change here, it will insert the go_package declaration if none of the provided .protos has it. If one of them has it, it will return, if all of the files have it then will be skipped.
example/no-gopackage will provided an example also a test for this new feature.

append the go_package automatically using fix_gopackage.sh.
fix unnecessary logic and remove unused functions.
move deep/bar under bar. remove pb.go here since it should be generated under protogen/
@jekiapp jekiapp changed the base branch from master to v1.11-beta February 2, 2022 10:30
@jekiapp jekiapp marked this pull request as ready for review February 2, 2022 10:40
fix typo in entrypoint.sh and integration-test.yml
@jekiapp jekiapp force-pushed the support_no_gopackage branch from ba944ca to dfe4706 Compare February 2, 2022 10:49
@HairyMike
Copy link
Contributor

This looks great @jekiapp 👍 .

Only thought I have is that the container size is pretty big and I thought it would be nice to try and move towards using a scratch container. If thats to happen we'd need to absorb the bash script's behaviour into the binary, but I'd push that into the future.

I'm looking forward to this one being released so we can get back onto latest.

@jekiapp
Copy link
Contributor Author

jekiapp commented Feb 3, 2022

@HairyMike Let me create the PR to master first to update the Readme, so that people use the beta build and test it before I merge this changes to latest.

@jekiapp
Copy link
Contributor Author

jekiapp commented Feb 8, 2022

After I build the image and the protoc plugin updated to the latest version it didn't behave like the previous version. I need to do a workaround on it first.

@jekiapp jekiapp changed the title [WIP] Support no gopackage Support no gopackage Feb 15, 2022
@jekiapp jekiapp merged commit 559ec91 into v1.11-beta Feb 15, 2022
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.

2 participants