We're currently using `since` and `to`, but `from:` and `to:` is probably better when refering to line numbers, not time. The suggestion is to add a parameter object (can be named `LineRange` or `LineSelection`). We can then use and pass it along instead of passing 2 separate params. ____ The following discussions from gitlab-ce!31361 should be addressed: - [ ] @patrickbajao started a [discussion](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31361#note_198605813): (+4 comments) > Hi @alexkalderimis, can you please review this MR? Thanks! :slight_smile: - [ ] @alexkalderimis started a [discussion](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31361#note_199834522): (+1 comment) > We also need a test for the `to:` parameter, wouldn't you agree? - [ ] @alexkalderimis started a [discussion](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31361#note_199834524): > Is the expectation that these values should only be provided together? If I supply a since _or_ a _to_, I would expect a performance speedup. Is that expectation met? > > Also I am beginning to wonder if it would not be helpful to package these two values, which are related, into a parameter object, perhaps something like `LineSelection.new(since, to)`. This would allow things like: > > ```ruby > all_lines[selection.to_range] > ``` > > and have meaningful methods such as `has_lower_bound?` and `has_upper_bound?` or even just `is_bounded?` and `is_all?`. > > Further to this, I have issues with the choice of `since` as a parameter name. "Since" is always used for time in English, and page/line numbers are always treated spatially. We say 'from line 2 to line 5'. Is there a reason why we should not use the appropriate preposition here? - [ ] @alexkalderimis started a [discussion](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31361#note_199834525): (+2 comments) > This is the kind of expression that a parameter object (e.g. `LineSelection` or `LineRange`) would make a bit nicer, allowing us to say: > > ```ruby > if bottom? && line_range.includes?(all_lines.size) > ``` > > Obviously this includes more indirection, but I feel it might make things a bit clearer. > > Thoughts? - [ ] @alexkalderimis started a [discussion](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31361#note_199834526): > see comment below about choice of parameter names and concept of a parameter object. It might be nicer to add a single parameter here rather than two.