Review comments API #83
Conversation
marcandre
force-pushed
the
no_comment
branch
3 times, most recently
from
7eeb933 to
2d7ec2d
Compare
|
@bbatsov, do you prefer I wait for your approval for this kind of PR? |
|
Sorry about the slow response. I was on vacation for a week and I'm behind on a ton of things. The changes seem fine to me.
If you're very confident about some changes you don't really need my approval. I don't want to be a bottleneck for progress. :-) |
| @@ -99,22 +99,39 @@ def blank? | |||
| ast.nil? | |||
| end | |||
|
|
|||
| # @return [Comment, nil] the comment at that line, if any. | |||
| def comment_at_line(line) | |||
There was a problem hiding this comment.
I guess this covers both whole line comments and inline comments, right?
There was a problem hiding this comment.
Yes! API that applies only to whole line comments should be pretty rare imo
Great, hope you had a good time! |
Only issue with this is I'm a very confident person 😅, and I don't always put enough emphasis on naming / doc. Given these, I'll merge #82 which I consider obvious, and I'll wait for #72 which is on hold because of the name of a module meant for internal use (your input appreciated)... |
Deprecate `ProcessedSource#comments_before_line`
This PR:
ProcessedSource#comment_at_lineProcessedSource#each_comment_in_linesIt deprecates
ProcessedSource#comments_before_line(which is imo a big code smell).I can't believe how much time I spent to arrive to this quite small PR; it's like version 5.0, I was really slow to get to this API.
I have reviewed / fixed all uses of any comment-accessing method in the main gem. The PR is upcomming. My main take is that any PR that autocorrects any range that could span more than one line must have specs with comments on each single line (close to that range).