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.

Sign up for GitHub

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

Jump to bottom

[Fix #1779] New cop ModuleLength #1788

Merged
merged 1 commit into from
Apr 21, 2015
Merged

[Fix #1779] New cop ModuleLength #1788

merged 1 commit into from
Apr 21, 2015

Conversation

sdeframond
Copy link
Contributor

Following discussion on #1779, here is a new cop that checks for overly long module definitions.

Since ModuleLength and ClassLength do almost the same thing, I grouped the common code into a new ClassishLength module.

Hope this helps.

@sdeframond sdeframond force-pushed the metrics/module_length branch from 91e3a2d to 250b11e Compare April 12, 2015 15:39
@sdeframond
Copy link
Contributor Author

Oops, seems like it has some issues. Please do not merge yet.

@sdeframond
Copy link
Contributor Author

Looking better now 😄. Go ahead!

@jonas054
Copy link
Collaborator

Please change the commit message to "[Fix #1779] New cop ModuleLength" (no dot at the end).

@sdeframond
Copy link
Contributor Author

Should I change the changelog entry as well?

On April 12, 2015 6:34:13 PM CEST, Jonas Arvidsson [email protected] wrote:

Please change the commit message to "[Fix #1779] New cop ModuleLength"
(no dot at the end).

Reply to this email directly or view it on GitHub:
#1788 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@sdeframond sdeframond force-pushed the metrics/module_length branch from 6f4d1fc to e07b1de Compare April 12, 2015 18:29
@sdeframond
Copy link
Contributor Author

Done.

@sdeframond sdeframond changed the title New cop ModuleLength. [Fix #1779] New cop ModuleLength Apr 13, 2015
@jonas054
Copy link
Collaborator

👍 Looks good to me!

@sdeframond
Copy link
Contributor Author

Hi, is there something blocking this feature? Should I rebase it on master?

@jonas054
Copy link
Collaborator

@sdeframond You need to rebase now.

@sdeframond sdeframond force-pushed the metrics/module_length branch from e07b1de to c8fff07 Compare April 21, 2015 13:11
@sdeframond
Copy link
Contributor Author

@jonas054 I just rebased.

bbatsov
bbatsov reviewed Apr 21, 2015
View reviewed changes
CHANGELOG.md
@@ -24,6 +24,7 @@

### New features

* [#1788](https://github.com/bbatsov/rubocop/pull/1788): New cop `ModuleLength` checks for overly long module definitions. ([@sdeframond][])
Copy link
Collaborator

Choose a reason for hiding this comment

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

This entry is not in the right place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, my bad. It should be OK now.

@sdeframond sdeframond force-pushed the metrics/module_length branch from c8fff07 to 7c50c12 Compare April 21, 2015 14:34
bbatsov added a commit that referenced this pull request Apr 21, 2015
@bbatsov
Merge pull request #1788 from sdeframond/metrics/module_length
29e8808 
[Fix  #1779] New cop ModuleLength
@bbatsov bbatsov merged commit 29e8808 into rubocop:master Apr 21, 2015
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 21, 2015

👍

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.
3 participants
@sdeframond @jonas054 @bbatsov