DSL refactor and minor Ruby improvements and fixes #8
Conversation
`Param` shouldn’t have to know about subclasses
joelvh
force-pushed
the
feature/refactor
branch
from
a7fa860 to
e55bbfe
Compare
joelvh
changed the title
|
@zverok I'm not getting the Rubocop errors that Travis is :-( |
It is the latest Rubocop's newly introduced cops, probably you should update it. |
examples/demo_base.rb
Outdated
| @@ -1,6 +1,6 @@ | |||
| require 'pp' | |||
|
|
|||
| $:.unshift 'lib' | |||
| $:.unshift '../lib' | |||
There was a problem hiding this comment.
Let's $.LOAD_PATH.unshift File.expand_path('../lib', __FILE__) or something.
Otherwise, this change will prohibit running ruby examples/example.rb from TLAW root.
lib/tlaw/dsl/base_wrapper.rb
Outdated
| # end | ||
| # ``` | ||
| # | ||
| # See also {#post_process} for some generic explanation of post-processing. |
There was a problem hiding this comment.
Please return these docs to dsl.rb :)
There was a problem hiding this comment.
Was a cherry-pick merge issue
lib/tlaw/params.rb
Outdated
| @@ -0,0 +1,38 @@ | |||
| module TLAW | |||
| # TODO: Add docs | |||
There was a problem hiding this comment.
Let's at least add something dummy, like "See Params::Base for explanations about params." or something?..
| end | ||
|
|
||
| private | ||
|
|
||
| def child(symbol, expected_class, **params) | ||
| children[symbol] | ||
| .tap { |child_class| | ||
| .tap do |child_class| |
There was a problem hiding this comment.
TBH, I prefer curly braces for "functional" method chains. I will not fight for it furiously, but if it is not a big deal for you,
tap {
...
}.new(...)...looks more natural for me, even in huge multiline chains.
There was a problem hiding this comment.
@zverok I've adopted the do blocks for multi-line because many other projects have Rubocops that require this style. I've come to prefer it, but will avoid changing them in this project for now :-)
|
@zverok updates made. That was odd that Rubocop changed defaults. Do you have a way to manage updating gems? Do you just update gems when running tests automatically? |
| text | ||
| .gsub(/^[ \t]+/, '') | ||
| .gsub(/\A\n|\n\s*\Z/, '') | ||
| end |
There was a problem hiding this comment.
The last thing that I noticed: let's probably remove this logic completely? It just repeats what Ruby 2.3 "squiggly heredoc" do, and as this PR specifies 2.3 as the minimum version, there is no need to replicating language features. One can just
description <<~D
Any description
goes here.
Like a boss!
D...and be happy.
It is not changed defaults, it is new cops added in new version. Probably tlaw's Gemfile just should specify a particular version of Rubocop, and I should update the version manually from time to time. |
|
Thank you for the awesome work! |
|
ok, great! |
@zverok here's the refactored changes without processor/transformation changes as your requested in #7.