The patch does not change the functionality of 'remove_columns', just bugfix, so i suppose no additional tests are required, are they?
2009/1/16 Peter Rabbitson <rabbit+l...@rabbit.us <rabbit%2bl...@rabbit.us>> > Oleg Pronin wrote: > > Maybe i misunderstood something but i suppose there is a bug in > > remove_columns. > > > > DB<14> x > > $this->columns > > > > 0 'id' > > ... > > 16 'logo_small' > > 17 'logo_mid' > > 18 'logo_full' > > 19 'screenshots' > > 20 'data' > > 21 'logo' > > > > $this->remove_column($c); # $c = 'logo'; > > > > DB<15> x > > $this->columns > > > > 0 'id' > > ... > > 16 'logo_mid' > > 17 'logo_full' > > 18 'screenshots' > > 19 'data' > > > > It removed 'logo_small' as well. > > > > In DBIx::Class::ResultSource: > > > > sub remove_columns { > > my ($self, @cols) = @_; > > > > > > return unless $self->_ordered_columns; > > > > my $columns = $self->_columns; > > my @remaining; > > > > foreach my $col (@{$self->_ordered_columns}) { > > push @remaining, $col unless grep(/$col/, @cols); > > > > } > > > > foreach (@cols) { > > delete $columns->{$_}; > > }; > > > > $self->_ordered_columns(\...@remaining); > > } > > > > > > What is this: "grep(/$col/, @cols);" and what for? > > > > Doc says > > "$table->remove_columns(qw/col1 col2 col3/); > > Removes columns from the result source." > > > > Not "Removes all columns that looks like" :-) > > > > Patch: > > > > sub remove_columns { > > my ($self, @to_remove) = @_; > > > > > > return unless my @ordered = $self->_ordered_columns; > > > > my $columns = $self->_columns; > > delete $columns->{$_} for @to_remove; > > > > my %to_remove = map {$_ => 1} @to_remove; > > $self->_ordered_columns([grep {!$to_remove{$_}} @ordered]); > > > > } > > > > Looks like a bug. Patch in the next email seems reasonable. However, > Oleg, you have been around long enough to know the drill - no patches > without tests. > > _______________________________________________ > List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class > IRC: irc.perl.org#dbix-class > SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/ > Searchable Archive: > http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk >
_______________________________________________ List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class IRC: irc.perl.org#dbix-class SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/ Searchable Archive: http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk