Skip to content

Commit

Permalink
Remove ArgumentError.
Browse files Browse the repository at this point in the history
It is untested. There is no `@frame` variable. Presumably it is supposed
to be `@framing`, but changing that shows that some of the tests *are*
setting frame twice.

I don't see why this level of strictness is necessary. If someone
disagrees, they should add a test for this behaviour and make the other
tests pass.
  • Loading branch information
jonleighton committed Jul 13, 2012
1 parent cbff1bc commit b543204
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions lib/arel/nodes/window.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ def order *expr
end

def frame(expr)
raise ArgumentError, "Window frame cannot be set more than once" if @frame
@framing = expr
end

Expand Down Expand Up @@ -75,4 +74,4 @@ def initialize(expr = nil)
end
end
end
end
end

2 comments on commit b543204

@jonleighton
Copy link
Member Author

Choose a reason for hiding this comment

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

@alexstaubo please check this commit ^^

@atombender
Copy link

Choose a reason for hiding this comment

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

Seems like a logical change for now. The reason you must be able to assign @framing is that you can call rows alone, or a subexpression of frame. I'd like to work on the API to make it more stateless.

(Sorry about the late reply, didn't see this until now.)

Please sign in to comment.