Skip to content

Commit

Permalink
Remove skip-empty warning; error instead.
Browse files Browse the repository at this point in the history
Rather than a soft deprecation here over two releases, we remove recognizing the flag entirely.

The migration path is small/easy for this change (updating the command to simply remove the option), and we don't have a very large user base at the moment.
  • Loading branch information
darronschall committed May 28, 2024
1 parent 707a597 commit ba15374
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 33 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## [Unreleased]

- Make `skip-empty` the default behavior; Deprecate flag (to be removed next version) - [#40](https://github.com/collectiveidea/protoc-gen-twirp_ruby/pull/40)
- Make `skip-empty` the default behavior; remove recognizing the option flag - [#40](https://github.com/collectiveidea/protoc-gen-twirp_ruby/pull/40)
- Update GitHub action to run specs on all supported Ruby versions - [#37](https://github.com/collectiveidea/protoc-gen-twirp_ruby/pull/37)

## [1.1.1] - 2024-05-22
Expand Down
23 changes: 5 additions & 18 deletions lib/twirp/protoc_plugin/process.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ class << self
def process(input)
request = Google::Protobuf::Compiler::CodeGeneratorRequest.decode(input)

options, warnings = extract_options(request.parameter)

warnings.each do |warning|
$stderr << "WARNING: #{warning}"
end
options = extract_options(request.parameter)

response = Google::Protobuf::Compiler::CodeGeneratorResponse.new
response.supported_features = Google::Protobuf::Compiler::CodeGeneratorResponse::Feature::FEATURE_PROTO3_OPTIONAL
Expand All @@ -49,17 +45,12 @@ def process(input)
#
# The only valid parameter is currently the optional "generate" parameter.
#
# @return [Array(Hash{Symbol => Symbol}, Array<String>?)] the extracted options as the first
# element, and an array of any applicable warnings as the second element (or nil when no
# warnings are present).
#
# The first return value is a Hash that contains:
# * :generate [Symbol] one of: :service, :client, or :both. Default :both.
# @return Hash{Symbol => Symbol} the extracted options, a Hash that contains:
# * :generate [Symbol] one of: :service, :client, or :both. Default :both.
#
# @raise [ArgumentError] when a required parameter is missing, a parameter value is invalid, or
# an unrecognized parameter is present on the command line
def extract_options(params)
warnings = []
opts = {
generate: :both
}
Expand All @@ -69,11 +60,7 @@ def extract_options(params)
# In the event value contains an =, we want to leave that intact.
# Limit the split to just separate the key out.
key, value = param.split("=", 2)
if key == "skip-empty"
# Ignore; this is now the default behavior. A future release will remove
# this flag entirely, and will raise an error instead of a warning.
warnings << "The `skip-empty` flag is deprecated and will be removed next release; it is now the default behavior."
elsif key == "generate"
if key == "generate"
if value.nil? || value.empty?
raise ArgumentError, "Unexpected missing value for generate option. Please supply one of: service, client, both."
end
Expand All @@ -89,7 +76,7 @@ def extract_options(params)
end
end

[opts, warnings]
opts
end
end
end
Expand Down
Binary file not shown.
14 changes: 0 additions & 14 deletions spec/twirp/protoc_plugin/process_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,6 @@
end
end

context "when passing the `skip-empty` flag" do
# Generate code gen request fixture:
# `./spec/support/create_fixture -b -p skip-empty -f service_code_gen_request_skip_empty_flag_pb.bin ./spec/fixtures/service.proto`
let(:request_pb) { fixture("service_code_gen_request_skip_empty_flag_pb.bin").read }

it "outputs a deprecation warning to stderr" do
allow($stderr).to receive(:<<)

Twirp::ProtocPlugin.process(request_pb)

expect($stderr).to have_received(:<<).with("WARNING: The `skip-empty` flag is deprecated and will be removed next release; it is now the default behavior.")
end
end

context "when passing an empty value for the generate flag" do
# Generate code gen request fixture:
# `./spec/support/create_fixture -b -p generate= -f service_code_gen_request_generate_param_missing_value_pb.bin ./spec/fixtures/service.proto`
Expand Down

0 comments on commit ba15374

Please sign in to comment.