Introducing Rubocop #66
Conversation
abinashmeher999
force-pushed
the
rubocop
branch
from
e3958de
to
b0cbe39
Compare
Created by executing `rubocop --auto-gen-config` Currently since the .rubocop.yml file is empty it uses the default.yml settings as the default for the style-guide. Custom rules can be added into the local .rubocop.yml and it will override the default. `inherit_from: .rubocop_todo.yml` currently ignores the violations reported in .rubocop_todo.yml. Next step is to remove the exclusions in the todo file and fix the violations. This way all the corrections regarding a particular style can be added in a single commit. Repeat this till all exclusions are fixed.
abinashmeher999
force-pushed
the
rubocop
branch
from
40140a0
to
53002dd
Compare
|
@v0dro I referred to the discussion here: SciRuby/nmatrix#521 to set this up. Every violation passes for now. Could you please review? |
|
Yeah works. You can add rubocop to the Travis build so that it fails in case of violations and it can be easily seen on GitHub. |
This is triggered because method names shouldn't start with an UpperCase, but this instance is okay as it emulates a class
|
@abinashmeher999, can you enable edits from maintainers? |
|
Done. |
|
This is now ready for review |
|
@abinashmeher999, @zverok would you mind reviewing this please? It's +1 from me. |
|
Looking at it. |
| @@ -25,45 +25,53 @@ def symbols ary_or_string, *params | |||
|
|
|||
| # Make an Array of SymEngine::Symbols from the parameters we received, | |||
| # now that they're normalized. | |||
| ary_or_string.map do |symbol_or_string| | |||
| ary_or_string.map do |symbol_or_string| | |||
| SymEngine::Symbol.new(symbol_or_string) | |||
| end | |||
There was a problem hiding this comment.
ary_or_string.map(&SymEngine::Symbol.method(:new))
| subject { SymEngine.evalf(SymEngine.sin(SymEngine(2)), 53, true) } | ||
| it { is_expected.to be_a SymEngine::RealDouble } | ||
| it { is_expected.to be_within(1e-15).of(0.909297426825682) } | ||
| end |
There was a problem hiding this comment.
I see that. There are more such occurrences in this file alone where a blank line will increase the readability. Maybe further beautification(like fixing line length too) can be added with a new PR by limiting this PR to only get rubocop to a passing state.
| it { is_expected.to be_a SymEngine::ComplexMPC } | ||
| its(:imaginary) { is_expected.to be_within(SymEngine::RealMPFR.new("1e-32", 100)).of(SymEngine::RealMPFR.new("0.84147098480789650665250232163005", 100)) } | ||
| end | ||
| end |
| else | ||
| f.capitalize | ||
| end | ||
| } | ||
| end |
There was a problem hiding this comment.
To be honest, for me the rule "do..end for multiline blocks" could be disabled at least for spec/ folder.
There was a problem hiding this comment.
Either is okay with me.
|
|
||
| it_behaves_like 'two symbols' | ||
| end | ||
|
|
||
| context 'with a splatted argument' do | ||
| let(:args) { %w[x y] } # it would be several separate strings on *args | ||
| let(:args) { %w(x y) } # it would be several separate strings on *args |
There was a problem hiding this comment.
Again, matter of personal style, but it is easy to redefine in .rubocop.yml what is appropriate limitation characters for %w arrays. And for me, %w[] is much more readable.
| asinh: 'Math.asinh', acosh: 'Math.acosh', atanh: 'Math.atanh', | ||
| pi: 'Math::PI', E: 'Math::E', I: '::Complex::I', | ||
| dirichlet_eta: 'SymEngine::Utils::evalf_dirichlet_eta', | ||
| zeta: 'SymEngine::Utils::evalf_zeta', gamma: 'Math.gamma' }.map { |from, to| [/(\b#{from}\b)/, to] }.to_h.freeze |
There was a problem hiding this comment.
This line should be broken down, maybe when we are fixing exceeding line length.
|
Thanks @isuruf! |
|
Made the suggested changes. |
|
@abinashmeher999, thanks for the work and @zverok, @v0dro, @certik for the reviews |
No description provided.