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

Create accessor methods to XMLRPC::Config #40

Merged
merged 12 commits into from
Aug 24, 2023
Merged

Create accessor methods to XMLRPC::Config #40

merged 12 commits into from
Aug 24, 2023

Conversation

herwinw
Copy link
Member

@herwinw herwinw commented Aug 21, 2023

Currently the config is made up of a number of constants, changing the config does require some boilerplate to redefine constants. An example:

XMLRPC::Config.module_eval { remove_const(:ENABLE_NIL_PARSER) }
XMLRPC::Config::ENABLE_NIL_PARSER = true

This change adds accessor methods to the Config module, so you can simply write them like this:

XMLRPC::Config.enable_nil_parser = true

It still uses the constants under water, which means it is fully backwards compatible (which was the big objection in #39 )

:ENABLE_MULTICALL,
:ENABLE_INTROSPECTION
].each do |option|
define_singleton_method("#{option.downcase}") do
Copy link
Member

Choose a reason for hiding this comment

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

Could you append ? only for ENABLE_*?
For example, enable_nil_create? for :ENABLE_NIL_CREATE.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added predicate methods for all the boolean values, and updated the calling code to use the predicates instead.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks.
But do we need both of enable_nil_create and enable_nil_create??

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not. I've removed the enable_nil_create and friends for the boolean options.

end

define_singleton_method("#{option.downcase}=") do |value|
module_eval do
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this module_eval?

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out we don't. I copy-pasted this from some code we had for changing the config options, but that was run from another module than Config.

I'ts gone now.

@kou kou merged commit 9f219e4 into ruby:master Aug 24, 2023
16 checks passed
@kou
Copy link
Member

kou commented Aug 24, 2023

Thanks.
I've merged this.

@herwinw herwinw deleted the config branch August 24, 2023 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants