Conversation
jdleesmiller
left a comment
There was a problem hiding this comment.
Thanks for taking this on!
I have now read through. It may look like a lot of comments, but they are for a small number of repeated things.
4916556 to
430e2f4
Compare
Rather than turn it off for a whole file, it's better to mark these exceptions in comments.
This was kind of a misfire of this cop, but no bother to change.
430e2f4 to
ce17c57
Compare
|
I've done the |
jdleesmiller
left a comment
There was a problem hiding this comment.
OK, thanks. All looks good.
On the changes to strings / file names: in general, changing both tests and code at the same time makes me a bit nervous, so I'd lean toward making the minimal set of changes possible to the tests. However, I don't see any particular problems here; it seems unlikely that the case of the the files in the tests is a significant factor in the test suite coverage.
|
Thanks. Point taken about changing code and tests together; I agree. But as you say these changes are fairly safe in this instance... |
This PR goes through and cleans up rubocop naming cops that don't affect the API.
There were many variables/parameters/etc named in
camelCasestyle, possibly a hangover from the early days of this library and its Java-like roots. This PR renames them to ruby standardsnake_casestyle.Most names have been translated as simply as possible - e.g.
entryNametoentry_name- but some have been simplified as well - e.g.aProctoproc. There are no changes to functionality in this PR; any other changes, such as formatting, have only been done to conform to our existing rubocop standards.