Arild Shirazi ashirazi

  • Washington D.C.
  • Joined on
ashirazi commented on commit AEgan/HUTrader@9ab7a37220
@ashirazi

eager loading. nice.

ashirazi commented on commit AEgan/HUTrader@ae05cde2b8
@ashirazi

probably want to handle edge cases like 404 team not found

ashirazi commented on commit AEgan/HUTrader@0a9a1290aa
@ashirazi

Probably best to let ActiveRecord handle cascading deletes for your association (see note for team below). As a tangential aside, grabbing all and …

ashirazi commented on commit AEgan/HUTrader@0a9a1290aa
@ashirazi

(see comment on Player) has_many :players, dependent: :destroy

ashirazi commented on commit AEgan/HUTrader@ec5b912225
@ashirazi

seen this twice now... could become a before_action before_action my_account do if !logged_in? || current_user.id != @user.id flash[:warning] = "Yo…

ashirazi commented on commit AEgan/HUTrader@9142727a74
@ashirazi

Test description confusing... redirecting to user page, not home in this case.

ashirazi commented on commit AEgan/HUTrader@9142727a74
@ashirazi

The comment has since been removed.

ashirazi commented on commit AEgan/HUTrader@c7875c6016
@ashirazi

why this as opposed to reset_session ?

ashirazi commented on commit AEgan/HUTrader@8162dedfa6
@ashirazi

the comment would be better as part of the test name... (just my opinion) should "not create 2 users with the same name" do

ashirazi commented on commit AEgan/HUTrader@8162dedfa6
@ashirazi

I would suggest tests for non-200 responses also (invalid id's, etc).

ashirazi commented on commit AEgan/HUTrader@8162dedfa6
@ashirazi

excellent

ashirazi commented on commit AEgan/HUTrader@426f239fe3
@ashirazi

I like these types of tests.

ashirazi commented on commit AEgan/HUTrader@426f239fe3
@ashirazi

The last assertion is the important one. Alternatively, you could use: [@ryan, @john, @matt].each { |u| assert xbox_users.include? u }

ashirazi commented on commit AEgan/HUTrader@426f239fe3
@ashirazi

So generally tests are easier to maintain if you have only 1 assertion per test. I would suggest using only the assertion that best tests the descr…

ashirazi commented on commit AEgan/HUTrader@426f239fe3
@ashirazi

sweet... factories!

ashirazi commented on commit AEgan/HUTrader@426f239fe3
@ashirazi

I prefer enumerations as indexed strings over magic numbers... just makes it easier when reporting or otherwise dealing with the data. (just a pref…

ashirazi commented on commit AEgan/HUTrader@1509d67a1e
@ashirazi

I got something better for you :) I'll get back when I package my work in a gem.

ashirazi commented on commit AEgan/HUTrader@1509d67a1e
@ashirazi

I like this!!! (I use "refute").

ashirazi commented on commit AEgan/HUTrader@1509d67a1e
@ashirazi

probably more readable with :warn or :info, etc. rather than an integer.

ashirazi commented on commit AEgan/HUTrader@1509d67a1e
@ashirazi

There are benefits to using factories over fixtures, especially as the data & relations become more complex, or constraints are added/removed. If t…

ashirazi commented on commit AEgan/HUTrader@1509d67a1e
@ashirazi

A git squash would make things easier to review next time!

ashirazi commented on commit AEgan/HUTrader@1509d67a1e
@ashirazi

Probably just easier to use rspec. I used to prefer shoulda too, but it seems like the community has moved on to either mini-test or rspec.

ashirazi created repository ashirazi/return_to_sender