Place `Config::format` behind the `format` feature flag.

tokio-rs / prost

PROST! a Protocol Buffers implementation for the Rust Language

Apache License 2.0

3.69k stars 481 forks source link

Place `Config::format` behind the `format` feature flag. #1022

Closed gibbz00 closed 3 months ago

gibbz00 commented 4 months ago

Fixes: #1021

~~Depends upon: #1020.~~

caspermeijn commented 4 months ago

Please make this PR independent of #1020.

My reasoning:

The current diff is unreadable
The other PR is in draft.

gibbz00 commented 3 months ago

Valid points 😊

Though of doing this at first, I wanted to avoid having to fix the conflicts that would arise. Do you think that this will be merged before #1020? I wouldn't mind making this an independent PR is that's the case.

caspermeijn commented 3 months ago

Well, from my point of view:

1020 is in draft and this PR is not, so #1020 can't be merged first
This PR and #1020 are completely unrelated, other than the same author and potential merge conflict.
I can't execute a good review of this PR, so it either needs to be fixed or should be opened after the other merge.

The last commit of this branch seems like an improvement, so I think it makes sense to keep the PR open. I suggest making it independent and solve the merge conflict at a later time.

gibbz00 commented 3 months ago

Alrighty, a rebase later shouldn't be too hard anyway. I've now made this PR independent of the rest.

gibbz00 commented 3 months ago

Rebased and fixed suggestions!

caspermeijn commented 3 months ago

Thank you for your contribution

tokio-rs / prost

Place `Config::format` behind the `format` feature flag. #1022

1020 is in draft and this PR is not, so #1020 can't be merged first