brendon / ranked-model

An acts_as_sortable/acts_as_list replacement built for Rails 4+
https://github.com/mixonic/ranked-model
MIT License
1.09k stars 134 forks source link

fix assigned but unused variable warnings #156

Closed walf443 closed 5 years ago

walf443 commented 5 years ago

Testing with -w option output following warnings:

$ bundle exec  rspec -w
./spec/support/active_record.rb:151: warning: assigned but unused variable - primary_key
./spec/support/active_record.rb:4: warning: already initialized constant ROOT
./spec/support/active_record.rb:4: warning: previous definition of ROOT was here
./spec/support/active_record.rb:6: warning: already initialized constant DB_CONFIG
./spec/support/active_record.rb:6: warning: previous definition of DB_CONFIG was here

This warning means primary_key is considered just variable not "#primary_key=" method. So calling it with self. is required.

brendon commented 5 years ago

Thanks @walf443, good catch. And thank you again for all your work on this gem! I do very much appreciate it :)

It looks like we're loading this file twice (at least). Perhaps we should deal with that (RE the reinitialised constants) rather than conditionally set the constant? We could define those constants in the rspec support harness instead?

brendon commented 5 years ago

One failing test for some reason:

Duck
609  a rearrangement
610    with max value
611      
...
614      
615        should eq [201, 204, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, ... 382, 383, 384, 385, 386, 387, 388, 389, 390, 391, 392, 393, 394, 395, 396, 397, 398, 399, 400, 202] (FAILED - 1)
616    with min value

We sometimes get an intermittent test failure with Postgres. I've never been able to figure it out but it seems like a timing issue. I've rebuilt the test, so hopefully it passes.

walf443 commented 5 years ago

@brendon Test passed!!

brendon commented 5 years ago

Nice! One day I'd like to get to the bottom of that issue, but for today we can carry on :D