Skip to content

Fix Style/Alias offenses#758

Merged
jaredbeck merged 1 commit intopaper-trail-gem:masterfrom
magni-:rubocop_alias
Apr 8, 2016
Merged

Fix Style/Alias offenses#758
jaredbeck merged 1 commit intopaper-trail-gem:masterfrom
magni-:rubocop_alias

Conversation

@magni-
Copy link
Copy Markdown
Contributor

@magni- magni- commented Apr 4, 2016

From #752.

magni-
magni- reviewed Apr 4, 2016
View reviewed changes
Comment thread lib/paper_trail/has_paper_trail.rb

# Utility method for reifying. Anything executed inside the block will
# appear like a new record.
# rubocop: disable Style/Alias
Copy link
Copy Markdown
Contributor Author

@magni- magni- Apr 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reposting my original comment here:

But the Alias false positive is an issue with Rubocop not parsing the instance_eval blocks properly. Making Rubocop happy for that case would actually end up breaking things.

Copy link
Copy Markdown
Contributor Author

@magni- magni- Apr 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I submitted a fix to Rubocop here: rubocop/rubocop#3004

@magni-
Copy link
Copy Markdown
Contributor Author

magni- commented Apr 4, 2016

Not quite sure what is wrong with the build (the only failure is 2.3.0/AR4/Postgres), since everything passes locally for me with that configuration.

@jaredbeck
Copy link
Copy Markdown
Member

jaredbeck commented Apr 6, 2016

Please rebase.

@jaredbeck
Copy link
Copy Markdown
Member

jaredbeck commented Apr 8, 2016

Looks good.

All the new aliases are in the same lexical scope as the methods they alias, so if I understand the style guide (https://github.com/bbatsov/ruby-style-guide#alias-method-lexically) we're good there.

Dropping track_associations (without the question mark) could be considered a breaking change, but I doubt anyone's using it and our next release is a major version (5), so I guess it's OK as long as we add a line to the changelog about it.

I don't like inline linter comments, but these are temporary so I can live with it.

Thanks Paul!

@jaredbeck jaredbeck merged commit 9658088 into paper-trail-gem:master Apr 8, 2016
@magni- magni- deleted the rubocop_alias branch April 8, 2016 04:57
magni- added a commit to magni-/paper_trail that referenced this pull request Apr 8, 2016
@magni-
Update changelog for paper-trail-gem#758
b4639c8
jaredbeck added a commit that referenced this pull request Apr 8, 2016
@jaredbeck
Merge pull request #770 from magni-/update_changelog
5dd7448 
Update changelog for #758
@magni- magni- mentioned this pull request May 13, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@magni- @jaredbeck