Rubocop + fixes#598
Conversation
albertvaka
force-pushed
the
albertvaka/rubocop
branch
4 times, most recently
from
1fbee06 to
1646fc3
Compare
- Enables rubocop - Fixes rubocop ofenses - Fixes broken `facts_to_tags` on A5, which was broken because the tests didn't run because the syntax was not correct (uncovered by rubocop) - Adds a test for `facts_to_tags` on A6/A7.
albertvaka
force-pushed
the
albertvaka/rubocop
branch
from
1646fc3 to
a2f1e46
Compare
truthbk
left a comment
truthbk
left a comment
There was a problem hiding this comment.
This looks good to me, the fact the tests are green appears to corroborate that. I added a couple of notes on things I noticed but that may be ignored.
Great stuff!
|
|
||
| context 'compile errors for incorrect values' do | ||
| let(:params) {{ use_mount: 'heaps' }} | ||
| let(:params) { { use_mount: 'heaps' } } |
There was a problem hiding this comment.
you've typically replaced the double let(:params) { { ... } } with:
let(:params) do
{
// ...
}
end
I'm not sure if you'd rather keep this as-is for a single-liner, or if you'd like to use the new notation for consistency. I have no strong preference, but you have gone for the consistent approach in most places, so just pointing this out.
| end | ||
|
|
||
| it { | ||
| if RSpec::Support::OS.windows? |
There was a problem hiding this comment.
The windows-style carriage return is no longer an issue? I guess not since tests are green and you added windows to the CI, but worth asking I guess :)
There was a problem hiding this comment.
The PDK skel adds a .gitattributes files that makes git change line ends on all files on commit, so they are consistent across OSes.
| .with_content(%r{deb\s+https://apt.datadoghq.com/\s+stable\s+7}) | ||
| end | ||
| end | ||
| let(:params) { { agent_version: '7.15.1' } } |
There was a problem hiding this comment.
ditto about the let blocks and consistency; this happens in several places in this file. Your call.
| 'content' => /^dd_url: https:\/\/notaurl.datadoghq.com\n/, | ||
| )} | ||
| let(:params) do | ||
| { dd_url: 'https://notaurl.datadoghq.com', |
There was a problem hiding this comment.
very picky nit: I was expecting different alignment on the braces in this block here (for consistency). This happens in a few more places in this file.
There was a problem hiding this comment.
I used rubocops autoformatter, but I agree this is ugly. Rubcop seems to accept it both ways, let me check if I can change it so this is an offense.
| # it should install the mirror | ||
| context 'with manage_repo => true' do | ||
| let(:params){ {:manage_repo => true, :agent_major_version => 5} } | ||
| let(:params) { { manage_repo: true, agent_major_version: 5 } } |
- Enables rubocop - Fixes rubocop ofenses - Fixes broken `facts_to_tags` on A5, which was broken because the tests didn't run because the syntax was not correct (uncovered by rubocop) - Adds a test for `facts_to_tags` on A6/A7.
2 participants
facts_to_tagson A5, which was broken because the testsdidn't run because the syntax was not correct (uncovered by rubocop).
facts_to_tagson A6/A7.