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

Add new style empty_lines_except_namespace for module and class body #3560

Closed
wants to merge 1 commit into from
Closed

Add new style empty_lines_except_namespace for module and class body #3560

wants to merge 1 commit into from

Conversation

legendetm
Copy link
Contributor

@legendetm legendetm commented Sep 30, 2016

Adds new style empty_lines_except_namespace for Style/EmptyLinesAroundClassBody and Style/EmptyLinesAroundModuleBody. It is related to #3452, but does not fix it entirely.

The new style requires empty lines around class/module body except for namespace classes/modules

module Providers
  class ProviderA
    class Xml

      include Provides::Xml

      def to_xml
        ...
      end

    end
  end
end

If class/module has multiple classes/modules, then empty lines are required

module Providers

  class ProviderA

  end

  class ProviderB

  end

end

Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Used the same coding conventions as the rest of the project.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • All tests are passing.
  • The new code doesn't generate RuboCop offenses.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.

@deivid-rodriguez deivid-rodriguez mentioned this pull request Sep 30, 2016
10 tasks
@bbatsov
Copy link
Collaborator

bbatsov commented Oct 12, 2016

Why only a partial fix?

@legendetm
Copy link
Contributor Author

legendetm commented Oct 12, 2016

The related ticket #3452 requires 3 changes

  • Require empty lines except for namespaces (current PR)
module Providers
  class ProviderA
    class Xml

      include Provides::Xml

      def to_xml
        ...
      end

    end
  end
end
  • Require empty lines except for namespaces and non def method at the beginning of the body (this change is ready and commited, but not pushed)
module Providers
  class ProviderA
    class Xml
      include Provides::Xml

      def to_xml
        ...
      end

    end
  end
end
  • Require blank lines around def methods (not sure yet, in which cop I should implement that)
module Providers
  class ProviderA
    class Xml
      include Provides::Xml
      # TODO: require blank line before the def method definition
      def to_xml
        ...
      end

    end
  end
end

I planned to tackle each issue with a separate pull request. I can combine the first two issues.

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 12, 2016

Ah, got it. Guess I didn't read the ticket carefully enough.

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 12, 2016

Update the changelog and squash those two commits together and I'll have this merged.

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 13, 2016

I merged this manually.

@bbatsov bbatsov closed this Oct 13, 2016
Neodelf pushed a commit to Neodelf/rubocop that referenced this pull request Oct 15, 2016
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