Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TrivialAccessors accepts DSL-style writers. #963

Merged
merged 1 commit into from
Apr 7, 2014
Merged

TrivialAccessors accepts DSL-style writers. #963

merged 1 commit into from
Apr 7, 2014

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Apr 5, 2014

@bbatsov what do you think of this change?

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 6, 2014

Maybe this should be an option. Technically speaking it's still a trivial accessor and most people are unlikely to be creating DSLs, so they should explicitly state their intent.

@tamird
Copy link
Contributor Author

tamird commented Apr 6, 2014

@bbatsov done

@@ -388,6 +388,7 @@ TrailingComma:
TrivialAccessors:
ExactNameMatch: false
AllowPredicates: false
AllowDSLWriters: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should add a comment for that, as it's not obvious from the name what's the point of the option.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 7, 2014

The code looks good. Address my minor remarks and rebase on top of the current master.

@tamird
Copy link
Contributor Author

tamird commented Apr 7, 2014

ok, this should be good to go

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 7, 2014

Indeed. Thanks!

bbatsov added a commit that referenced this pull request Apr 7, 2014
`TrivialAccessors` accepts DSL-style writers.
@bbatsov bbatsov merged commit a20a808 into rubocop:master Apr 7, 2014
@tamird tamird deleted the dsl-writer branch April 7, 2014 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants