Rubocop Fixes

[dks17]

#1124
Folows the #1125
Fixes:

[Evan-M]

@dks17: Thanks for this!

Unfortunately, I had to rebase my branch against the latest HEAD of master, and I did a --force push! (Probably should not have done that, since others have access to this branch repo). I think in doing so, I've introduced some of these conflicts?

I've got a backup branch locally from before I did my rebase, so I'll try doing a force push of that back to the add_rubocop branch, to restore the previous state.

UPDATE: I manually resolved my conflicts (which were totally a result of failing to rebase against master/HEAD prior to pushing the branch). My merge conflict resolution on #1125 seems to have introduced the same merge conflict here on #1126. @dks17: Let me know if you need help resolving this...

This is the code from master/HEAD to use (merged from #1115):

devise_token_auth/app/models/devise_token_auth/concerns/user.rb

Lines 260 to 272 in a9c7ea6

	def clean_old_tokens
	if tokens.present? && max_client_tokens_exceeded?
	# Using Enumerable#sort_by on a Hash will typecast it into an associative
	# Array (i.e. an Array of key-value Array pairs). However, since Hashes
	# have an internal order in Ruby 1.9+, the resulting sorted associative
	# Array can be converted back into a Hash, while maintaining the sorted
	# order.
	self.tokens = tokens.sort_by { |_cid, v| v[:expiry] || v['expiry'] }.to_h
	# Since the tokens are sorted by expiry, shift the oldest client token
	# off the Hash until it no longer exceeds the maximum number of clients
	tokens.shift while max_client_tokens_exceeded?
	end

You may need to manually apply some of the fixes from the Cops you already fixed when resolving. E.g. the Layout/SpaceInsideHashLiteralBraces cop on line 267:

devise_token_auth/app/models/devise_token_auth/concerns/user.rb

Line 267 in a9c7ea6

self.tokens = tokens.sort_by { |_cid, v| v[:expiry] || v['expiry'] }.to_h

[Evan-M]

@dks17, could you update either the .rubocop_todo.yml and/or the .rubocop.yml for each cop that you resolved?

The .rubocop_todo.yml (created via rubocop --auto-gen-config --exclude-limit 35) excludes files with offenses for each cop violation. Moreover, since all the cops listed are already enabled by default, you will most likely only need to remove the entire declaration for the resolved/fixed cop from the .rubocop_todo.yml. If you specified / or changed any of the default options for a given cop, then you must also create an entry for that cop in the .rubocop.yml file. I hope this makes sense.

Once the conflicts are resolved, I can merge this onto the add_rubocop branch. (The entire add_rubocop branch and PR #1125 can be reviewed more thoroughly once the cops all pass)

[dks17]

@Evan-M, I encountered only one conflict. It is OK now. I hope I did all properly.

[Evan-M]

I was actually thinking that instead of re-running the auto-gen, you would just manually remove the specific cops from the .rubocop_todo.yml. However, it is curious to me that when you ran rubocop --auto-gen-config --exclude-limit 35 the resulting .rubocop_todo.yml file re-enabled some of the cops that I'd fixed already...

If everyone's run of rubocop is non-deterministic, then this doesn't all quite work right?

What version of ruby are you running rubocop on? I'm using 2.4.2p198 on MacOS 10.13.3 (High Sierra).

I hope I did all properly.

Hey, no worries! We're figuring this out as we go, right? ¯\_(ツ)_/¯

[Evan-M]

@dks17 I'm going to go ahead and merge this into PR #1125.

We'll figure out the specific cop settings afterwards.

[dks17]

What version of ruby are you running rubocop on? I'm using 2.4.2p198 on MacOS 10.13.3 (High Sierra).

@Evan-M, I am using ruby 2.5.0p0 (2017-12-25 revision 61468). I think that is not matter in this case.