Rubocop friendly cops part 10 #4264
Conversation
DavidKang
added
DO NOT MERGE ⚠️
DavidKang
requested review from
evanrolfe,
eduardoj,
hennevogel,
bgeuken,
ChrisBr,
mschnitzer,
mdeniz and
Ana06
|
I created a separate PR (#4265) for having master without errors. Please remove your last commit, now it will be unneeded. |
|
LGTM overall, but I have some doubts about the Fix Style/LineEndConcatenation cop, is what we want? |
| @@ -302,9 +302,9 @@ class NoMatchingReleaseTarget < APIException | |||
| def verify_can_modify_target_package! | |||
| return if User.current.can_modify_package?(@package) | |||
|
|
|||
| raise CmdExecutionNoPermission, "no permission to execute command '#{params[:cmd]}' " + | |||
| raise CmdExecutionNoPermission, "no permission to execute command '#{params[:cmd]}' " \ | |||
There was a problem hiding this comment.
I can live with this change, but I don't really understand it... I need to look up the reasoning for why a backslash is preferred here.
DavidKang
force-pushed
the
feature/rubocop/style_cop_4
branch
from
fd651eb
to
f3df58f
Compare
Codecov Report
@@ Coverage Diff @@
## master #4264 +/- ##
==========================================
+ Coverage 87.8% 87.98% +0.17%
==========================================
Files 340 340
Lines 18588 18463 -125
==========================================
- Hits 16321 16244 -77
+ Misses 2267 2219 -48
|
|
@mdeniz, thanks 😸 |
DavidKang
force-pushed
the
feature/rubocop/style_cop_4
branch
from
f3df58f
to
b767182
Compare
There was a problem hiding this comment.
@DavidKang in e85fd02 it should be frontend] Fix Style/HashSyntax offenses (in plural).
I think it is a good practise to mention in the commit message that the offenses were autocorrected. 
Apart from that LGTM.
| if dp.error | ||
| currentpack['problems'] << 'error-' + dp.error | ||
| end | ||
| currentpack['problems'] << 'error-' + dp.error if dp.error |
There was a problem hiding this comment.
[nitpick]
Guess it is autocorrected but what about string interpolation?
"error-#{dp.error}"
| if @submits.has_key? key | ||
| currentpack['requests_to'].concat(@submits[key]) | ||
| end | ||
| currentpack['requests_to'].concat(@submits[key]) if @submits.has_key? key |
There was a problem hiding this comment.
[nitpick]
Guess it is autocorrected but what about parentheses?
@submits.has_key?(key)
(rubocop/rubocop#3224 has_key? seems to be "deprecated")
| if splitted.length > 2 | ||
| prjname += ':' + splitted[-1] | ||
| end | ||
| prjname += ':' + splitted[-1] if splitted.length > 2 |
src/api/app/models/bs_request.rb
Outdated
| if actions.is_a? Hash | ||
| actions = [actions] | ||
| end | ||
| actions = [actions] if actions.is_a? Hash |
DavidKang
force-pushed
the
feature/rubocop/style_cop_4
branch
from
b767182
to
dcadfbd
Compare
DavidKang
dismissed
Ana06’s stale review
@Ana06, I corrected the commit message. about the mention in the commit message that was autocorrected, I prefer to mention it when they are manually corrected.
DavidKang
removed
the
DO NOT MERGE ⚠️
@openSUSE/open-build-service Please review
The affected cops are:
Pair-programmed with: @bgeuken