Refactor GistTag

[ashmaroli]

• GistTag#context_contains_key? uses both :has_key? and key?. So its effectively a stalemate.
• GistTag.client is a public_class_method. It should be placed above the private access modifier
  

  jekyll-gist/lib/jekyll-gist/gist_tag.rb

  Line 97 in 1ca4a4d

  gist = GistTag.client.gist gist_id

[DirtyF]

As https://pages.github.com/versions/ supports it, I'm fine with requiring Liquid 4.0.
That's up to someone from GitHub to decide.
/cc @parkr

[parkr]

Yeah, requiring Liquid 4 is fine by me, but give a major bump to this gem if you do that. For now, backwards compatibility is good.

[DirtyF]

LGTM.

[ashmaroli]

@mattr-, @parkr I'd like to move the following regex to a private_constant. What would be a suitable name for the constant?

jekyll-gist/lib/jekyll-gist/gist_tag.rb

Line 49 in c74391b

matched = input.match(%r!\A([\S]+|.*(?=\/).+)\s?(\S*)\Z!)

[mattr-]

I wouldn't bother moving it to a constant at all. Very little gain for that change when it's already in a method that describes what's going on. Moving the regexp to a constant makes it more important in the class than it is. That's not the class' main concern, so best to leave it hidden inside that method, IMO.

[mattr-]

For a PR that was only supposed to be about rubocop offenses, this is starting to get off track. Let's cap it off here and then do further work in other PRs if necessary.

[ashmaroli]

@mattr- It had stopped being about RuboCop offenses much before I renamed the PR title. But I agree, its time to defer any more changes.

[DirtyF]

@jekyllbot: merge +dev