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

Fix copy table index test; Change == to ! on false in travis.rb #9847

Merged
merged 1 commit into from
Mar 21, 2013

Conversation

vipulnsward
Copy link
Member

  1. Fix copy table index test, that was wrongly testing same thing
  2. Change == to ! on false value in travis.rb

@vipulnsward
Copy link
Member Author

cc @carlosantoniodasilva

Not sure, if I should have removed the extra hash form comments.

@prathamesh-sonpatki
Copy link
Member

@vipulnsward What do you think about this?

failures = results.reject { |key, value| value }

@@ -117,7 +117,7 @@ def rake(*tasks)
# puts " Local gems:"
# `gem list`.each_line {|line| print " #{line}"}

failures = results.select { |key, value| value == false }
failures = results.select { |key, value| !value }
Copy link
Member

Choose a reason for hiding this comment

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

I think this is checking false to avoid keys with nil value to be a failure

Copy link
Member Author

Choose a reason for hiding this comment

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

The unless in the rake method handles nils

rafaelfranca added a commit that referenced this pull request Mar 21, 2013
Fix copy table index test; Change == to ! on false in travis.rb
@rafaelfranca rafaelfranca merged commit b1b9de3 into rails:master Mar 21, 2013
@rafaelfranca
Copy link
Member

Thank you ❤️ 💚 💙 💛 💜

@vipulnsward vipulnsward deleted the fix_sqlite_test branch March 21, 2013 12:34
@vipulnsward
Copy link
Member Author

😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants