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

Remove comment annotations #418

Closed
wants to merge 1 commit into from
Closed

Remove comment annotations #418

wants to merge 1 commit into from

Conversation

croaky
Copy link

@croaky croaky commented Mar 5, 2015

This is the only subsection of the guide,
which is perhaps a smell.

Its recommendations conflict with
earlier recommendations in the "Comments" section:

Write self-documenting code and ignore the rest of this section.
Avoid writing comments to explain bad code.
Refactor the code to make it self-explanatory.
Keep existing comments up-to-date.
An outdated comment is worse than no comment at all.

Each of TODO, FIXME, OPTIMIZE, HACK, and REVIEW
promote bad habits,
which can be better addressed by the earlier "Comments" section
or by using a project management system,
benchmarking tools,
or code review tools.

This is the only subsection of the guide,
which is perhaps a smell.

Its recommendations conflict with
earlier recommendations in the "Comments" section:

> Write self-documenting code and ignore the rest of this section.
> Avoid writing comments to explain bad code.
> Refactor the code to make it self-explanatory.
> Keep existing comments up-to-date.
> An outdated comment is worse than no comment at all.

Each of `TODO`, `FIXME`, `OPTIMIZE`, `HACK`, and `REVIEW`
promote bad habits,
which can be better addressed by the earlier "Comments" section
or by using a project management system,
benchmarking tools,
or code review tools.
@croaky
Copy link
Author

croaky commented Mar 5, 2015

Just an idea! Let me know what you think. Cheers.

@johana-star
Copy link
Contributor

I like this. 👍

@NewAlexandria
Copy link
Contributor

My only issue with this change is that code comment will occur, regardless of whether guidance for them is removed. Semantic notes are easier to collect and report on.

In the most pathological cases, the arrogance of smart engineers is tiresome and alienating to lesser-yet-indispensable engineers. Also, all coders 'great and small' are composed of many facets, some better than others. Someone who has a good semantically-common way of flagging their mistakes, then ultimately writes code that is easier to refactor.

@johana-star
Copy link
Contributor

@NewAlexandria I hear what you are saying, and it's certainly true that clever coders may have things in a working state, but still improvable with a code comment saying TODO or OPTIMIZE.

Would it be preferable to note that there are times where semantic notes will occur, and when they do they should follow these patterns, though when coders have a bug tracking system it is better to use that instead of semantic comments?

@NewAlexandria
Copy link
Contributor

@strand Maybe. Maybe for small outfits.

Bug tracking system are notorious for being the only way that upper-management knows what's going on. In many cases, excess 'low/no priority' bugs, or similar stories, create distractions that aren't needed: a good tech manager should run reports and keep their own collection in a wiki or similar that the whole team owns. Worthwhile things maybe get raised in retrospective or planning, and thrown on the backlog at that time.

All of this, IMO, is guidance beyond this style guide.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 2, 2017

Seems few people got involved in the discussion, so I'll close this PR. I hate the comment annotations myself, but I think at least the first few rules about styling them do belong to this document.

@bbatsov bbatsov closed this Jan 2, 2017
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.

4 participants