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

Documentation followed by annotation comments #1090

Closed
agrimm opened this issue May 16, 2014 · 5 comments
Closed

Documentation followed by annotation comments #1090

agrimm opened this issue May 16, 2014 · 5 comments
Assignees
Labels

Comments

@agrimm
Copy link
Contributor

agrimm commented May 16, 2014

Currently, the following code creates a Documentation offense:

# Does fooing.
# FIXME: Not yet implemented.
class Foo
  def initialize
  end
end

This offense can be avoided by putting the FIXME line (the annotation comment) first.

Is this deliberate, or unintentional?

I assume that the annotation comment is about the implementation, while the non-annotation comment is about interface/intention, and that the latter is more likely to be relevant to someone reading the file. Therefore, non-annotation comments should appear first.

@bbatsov
Copy link
Collaborator

bbatsov commented May 16, 2014

Seems like a bug to me. I guess the intention was to exclude comment annotations from class/module documentation. @jonas054 should know more about this.

@jonas054 jonas054 self-assigned this May 16, 2014
@jonas054 jonas054 added the bug label May 29, 2014
@jonas054
Copy link
Collaborator

Yes, it's bug. It should be the other way around IMO so that

# Does fooing.
# FIXME: Not yet implemented.
class Foo

is accepted, but

# FIXME: Not yet implemented.
# Does fooing.
class Foo

is not. It's very difficult to judge where the annotation ends, so I want to call the whole comment an annotation if the first line has an annotation keyword.

@jonas054
Copy link
Collaborator

I just saw that or own OneLineConditional does not live up to my new definition:
https://github.com/bbatsov/rubocop/blob/master/lib/rubocop/cop/style/one_line_conditional.rb#L6-L7

It would be confusing for users to get the Missing top-level class documentation comment report in such cases. Some more explanation would be needed in the message.

So I'm thinking now that we should abondon the idea of trying to disregard the annotation comments in this cop, and just accept any comment. What do you think, @bbatsov @yujinakayama @agrimm?

@bbatsov
Copy link
Collaborator

bbatsov commented May 30, 2014

@jonas054 Guess we can assume annotation comments are one line long.

@jonas054
Copy link
Collaborator

@bbatsov Good idea. I've done so in the PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants