AutoRemoveColumns _should_column_fetch example incorrect

[abraxxa]

Using https://metacpan.org/pod/DBIx::Class::Helper::ResultSet::AutoRemoveColumns#_should_column_fetch results in Can't locate object method "column_info" via package "My::Schema::ResultSet::Foo" because the ->result_source is missing.

[abraxxa]

And the code is logically also incorrect. This doesn't always return 1:

sub _should_column_fetch {
    my ( $self, $column ) = @_;

    my $info = $self->result_source->column_info($column);

    return exists $info->{remove_column}
      ? $info->{remove_column}
      : 0;
}

[abraxxa]

One more thing: the sub name _should_column_fetch suggests that it has to return a false value to prevent a column from being fetched while the opposite is true (pun intended).

[frioux]

please provide a pull request and a test.

On Sun, Apr 11, 2021, 4:36 PM Alexander Hartmaier @.***> wrote:

One more thing: the sub name _should_column_fetch suggests that it has to return a false value to prevent a column from being fetched while the opposite is true (pun intended).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/issues/106#issuecomment-817392969, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAB7Y6JZU4Q7GECHR5TJVLTIIW7JANCNFSM42YF4ZJQ .

[abraxxa]

I've created the PR, Github seems to not link it because of GH#106 instead of just #106. No test because it's just a doc patch.

Renaming the sub would introduce a breaking change so I haven't done that. Maybe adding documentation to clearly describe the expected return values can be done as a workaround.

[frioux]

Awesome, thanks

[abraxxa]

Sorry for the late reply, it seems I've left the brower tab open and didn't hit 'Comment':

One more thing: the sub name _should_column_fetch suggests that it has to return a false value to prevent a column from being fetched while the opposite is true (pun intended).