Add dataset filter methods by WorstOfAny · Pull Request #26 · AlexWayfer/sequel-enum_values

WorstOfAny · 2020-05-18T18:42:23Z

Want to have methods that will filter dataset by enum values

AlexWayfer · 2020-05-26T16:07:31Z

I want a configuration option for this, like for predicate_methods, with false default. Can you make it please?

AlexWayfer · 2020-06-25T10:15:21Z

Friendly ping @WorstOfAny

AlexWayfer · 2020-07-10T16:37:04Z

@WorstOfAny another friendly reminder.

WorstOfAny · 2020-07-20T00:38:17Z

@AlexWayfer sorry for delay. I have added the option you want.

I havent seen 'test container' checks that failed before. Tip me how to fix it pls.

AlexWayfer · 2020-07-20T15:55:11Z

@AlexWayfer sorry for delay.

No problem.

I have added the option you want.

Thank you.

I havent seen 'test container' checks that failed before. Tip me how to fix it pls.

It's not your fault. As you can see, for example, here tests are passed, but there is a report to Codecov which failed.

We have many issues with Codecov in these latter days, this one is like codecov/codecov-ruby#69 (comment)

I'll try to fix it separately.

AlexWayfer

Please, add documentation into README.

lib/sequel/plugins/enum_values.rb

spec/sequel/plugins/enum_values_spec.rb

AlexWayfer · 2020-07-20T16:26:21Z

I havent seen 'test container' checks that failed before. Tip me how to fix it pls.

It's not your fault. As you can see, for example, here tests are passed, but there is a report to Codecov which failed.

We have many issues with Codecov in these latter days, this one is like codecov/codecov-ruby#69 (comment)

I'll try to fix it separately.

Should be fixed in #35. Please, rebase the branch to master.

AlexWayfer · 2020-08-25T20:01:52Z

@WorstOfAny friendly reminder.

AlexWayfer · 2020-09-18T21:52:37Z

@WorstOfAny another friendly reminder, sorry. 🙈

WorstOfAny · 2020-09-21T17:28:00Z

Please, add documentation into README.

Added

AlexWayfer

Looks good with small notes. Also, please, remember about our discussion about redefining methods.

lib/sequel/plugins/enum_values.rb

sequel-enum_values.gemspec

spec/sequel/plugins/enum_values_spec.rb

AlexWayfer · 2020-09-21T18:18:34Z

I think I'll drop Ruby 2.4 in master (about CI fail).

README.md

AlexWayfer · 2020-09-25T12:43:49Z

I think I'll drop Ruby 2.4 in master (about CI fail).

Done.

AlexWayfer · 2020-10-15T16:55:43Z

Another friendly reminder.

AlexWayfer · 2020-11-16T21:23:35Z

Yet another friendly reminder (almost 2 months since the last update).

AlexWayfer · 2021-03-06T20:28:26Z

Yet another friendly reminder (almost 2 months since the last update).

Yet another reminder after almost 4 month.

AlexWayfer · 2021-04-02T12:06:18Z

I see conflicting files, can you resolve it or should I do this?

WorstOfAny · 2021-04-02T12:15:36Z

I see conflicting files, can you resolve it or should I do this?

sry, didn't notice this. Resolved

AlexWayfer

Thank you for returning to this PR.

AlexWayfer · 2021-04-02T12:12:03Z

README.md

+Plugin can define dataset methods for all enum values:
+
+```ruby
+Item.plugin :enum_values, filter_methods: true # default is `false`


A small naming suggestion, we can discuss it, but this is what I saw:

You're calling them dataset methods, it's actually Dataset's methods, but you named the option filter_methods. A bit confusing, and I see no problem with dataset_methods option: I don't think that there will be other dataset methods, and it's all about enums.

What do you think?

i can do it, but for me it's like change name of predicate_methods to instance_methods. We can't filter an instance, only collection or dataset, so that's why filter_methods was used

i thought that dataset_methods isn't clear enough naming.

My response (with agreement and suggestion) somwhy moved below: #26 (comment)

AlexWayfer · 2021-04-02T12:15:49Z

lib/sequel/plugins/enum_values.rb

-						Array(predicate_methods)
+				def process_enum_values_options
+					@enum_values_caching =
+						@enum_values_options.fetch(:caching, @enum_values_caching)


It fits in single line, as I see.

AlexWayfer · 2021-04-02T12:17:34Z

lib/sequel/plugins/enum_values.rb

+
+					process_enum_values_option(:filter_methods) do |field, value|
+						dataset.methods.include?(value.to_sym) &&
+							warn("WARNING: Redefining method #{value}")


Approach with && for condition very unclear, please use if/unless.

I see no reason for parentheses of warn.

It's a pretty good idea to warn, but I suggest to make it even strict: let raise an exception! And see what will happens with users.

And there are should be tests for this behavior!

AlexWayfer · 2021-04-02T12:18:59Z

lib/sequel/plugins/enum_values.rb

+					end
+
+					process_enum_values_option(:predicate_methods) do |field, value|
+						define_method("#{value}?", -> { public_send(field) == value })


Suggested change

define_method("#{value}?", -> { public_send(field) == value })

define_method("#{value}?") { public_send(field) == value }

But is here a really need to shorten this non-related code?

AlexWayfer · 2021-04-02T13:24:48Z

README.md

+### Filter methods
+
+Plugin can define dataset methods for all enum values:
+
+```ruby
+Item.plugin :enum_values, filter_methods: true # default is `false`


OK, I agree, what about such changes then?

Suggested change

### Filter methods

Plugin can define dataset methods for all enum values:

```ruby

Item.plugin :enum_values, filter_methods: true # default is `false`

### Dataset filter methods

Plugin can define dataset filter methods for all enum values:

```ruby

Item.plugin :enum_values, filter_methods: true # default is `false`

AlexWayfer · 2021-05-12T18:40:37Z

Do you want some help? 🙄

AlexWayfer · 2021-08-28T20:12:12Z

@WorstOfAny 😥

WorstOfAny force-pushed the add_dataset_filter_methods branch 5 times, most recently from e688bd3 to bd50baa Compare May 18, 2020 20:03

WorstOfAny force-pushed the add_dataset_filter_methods branch 8 times, most recently from 63787ec to 5342a93 Compare July 20, 2020 00:33

WorstOfAny force-pushed the add_dataset_filter_methods branch from 5342a93 to 6227605 Compare July 20, 2020 00:53

AlexWayfer requested changes Jul 20, 2020

View reviewed changes

lib/sequel/plugins/enum_values.rb Outdated Show resolved Hide resolved

lib/sequel/plugins/enum_values.rb Outdated Show resolved Hide resolved

spec/sequel/plugins/enum_values_spec.rb Outdated Show resolved Hide resolved

spec/sequel/plugins/enum_values_spec.rb Outdated Show resolved Hide resolved

WorstOfAny force-pushed the add_dataset_filter_methods branch 4 times, most recently from aa328d9 to c974218 Compare September 21, 2020 17:25

WorstOfAny force-pushed the add_dataset_filter_methods branch from c974218 to 0809727 Compare September 21, 2020 17:58

AlexWayfer requested changes Sep 21, 2020

View reviewed changes

AlexWayfer reviewed Sep 21, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

WorstOfAny force-pushed the add_dataset_filter_methods branch 2 times, most recently from f3fbbc3 to a2039b5 Compare September 22, 2020 20:23

WorstOfAny force-pushed the add_dataset_filter_methods branch from a2039b5 to 62ffd96 Compare March 30, 2021 14:30

WorstOfAny requested a review from AlexWayfer March 30, 2021 14:35

WorstOfAny force-pushed the add_dataset_filter_methods branch from 62ffd96 to a034894 Compare March 30, 2021 14:37

Add dataset filter methods

2acc579

WorstOfAny force-pushed the add_dataset_filter_methods branch from a034894 to 2acc579 Compare April 2, 2021 12:14

AlexWayfer requested changes Apr 2, 2021

View reviewed changes

AlexWayfer reviewed Apr 2, 2021

View reviewed changes

	define_method("#{value}?", -> { public_send(field) == value })
	define_method("#{value}?") { public_send(field) == value }

Conversation

WorstOfAny commented May 18, 2020

Uh oh!

AlexWayfer commented May 26, 2020

Uh oh!

AlexWayfer commented Jun 25, 2020

Uh oh!

AlexWayfer commented Jul 10, 2020

Uh oh!

WorstOfAny commented Jul 20, 2020

Uh oh!

AlexWayfer commented Jul 20, 2020

Uh oh!

AlexWayfer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexWayfer commented Jul 20, 2020

Uh oh!

AlexWayfer commented Aug 25, 2020

Uh oh!

AlexWayfer commented Sep 18, 2020

Uh oh!

WorstOfAny commented Sep 21, 2020

Uh oh!

AlexWayfer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexWayfer commented Sep 21, 2020

Uh oh!

Uh oh!

AlexWayfer commented Sep 25, 2020

Uh oh!

AlexWayfer commented Oct 15, 2020

Uh oh!

AlexWayfer commented Nov 16, 2020

Uh oh!

AlexWayfer commented Mar 6, 2021

Uh oh!

AlexWayfer commented Apr 2, 2021

Uh oh!

WorstOfAny commented Apr 2, 2021

Uh oh!

AlexWayfer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWayfer commented May 12, 2021

Uh oh!

AlexWayfer commented Aug 28, 2021

Uh oh!

Reviewers

Assignees