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

fix: handle camel case package #52

Merged
merged 1 commit into from
Jun 2, 2022
Merged

fix: handle camel case package #52

merged 1 commit into from
Jun 2, 2022

Conversation

cobbinma
Copy link
Contributor

Issue 51

convert package name to snake case when writing file to be consistent with prost_build 🐍

Copy link
Contributor

@tustvold tustvold left a comment

Choose a reason for hiding this comment

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

LGTM, thank you 👍

I'm guessing this just impacts the name of the files, and not anything else?

Also to clarify this is technically a breaking change, as whilst not consistent with prost_build, it still did work for such packages?

@cobbinma
Copy link
Contributor Author

cobbinma commented Jun 2, 2022

Yes, this should only impact the file names.

I am generating a mod.rs file after using tonic_build and pbjson_build. The json implementations are placed in the same module based on the file name.

pub mod google {
    pub mod pubsub {
        pub mod v1 {
           include!("google.pubsub.v1.rs")
           include!("google.pubsub.v1.serde.rs")
        }
    }
}

If the file names are different from tonic_build the approach will not work as both files need to be in the same module.

I could probably use the a custom writer to get around this but thought this was better default behaviour.

@tustvold tustvold added automerge Put in the merge queue kodiak: merge.method = 'squash' Instruct kodiak to perform a squash merge labels Jun 2, 2022
@kodiakhq kodiakhq bot merged commit b6af2a3 into influxdata:main Jun 2, 2022
@cobbinma cobbinma deleted the handle-camel branch June 2, 2022 09:26
@tustvold tustvold mentioned this pull request Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Put in the merge queue kodiak: merge.method = 'squash' Instruct kodiak to perform a squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants