Automatically annotating files upon running migrations increases diff noise

In commit 8278b763 the behaviour of the annotation Gem was changed to now run automatically when running migrations. This in turn results in a ton of useless changes being added whenever you run a DB migration. For example, I wanted to test a migration for !3860 (merged) and running rake db:migrate produced the following diff stat:

 app/models/appearance.rb                           |  12 +-
 app/models/application_setting.rb                  |  10 +-
 app/models/audit_event.rb                          |   4 +-
 app/models/broadcast_message.rb                    |   8 +-
 app/models/ci/build.rb                             |  15 +-
 app/models/ci/commit.rb                            |   6 +-
 app/models/ci/runner.rb                            |  14 +-
 app/models/ci/trigger.rb                           |   2 +-
 app/models/ci/variable.rb                          |   6 +-
 app/models/commit_status.rb                        |  15 +-
 app/models/deploy_key.rb                           |  10 +-
 app/models/deploy_keys_project.rb                  |   4 +-
 app/models/email.rb                                |   2 +-
 app/models/event.rb                                |   8 +-
 app/models/forked_project_link.rb                  |   4 +-
 app/models/generic_commit_status.rb                |  15 +-
 app/models/group.rb                                |  15 +-
 app/models/hooks/project_hook.rb                   |  15 +-
 app/models/hooks/service_hook.rb                   |  15 +-
 app/models/hooks/system_hook.rb                    |  15 +-
 app/models/hooks/web_hook.rb                       |  15 +-
 app/models/identity.rb                             |   4 +-
 app/models/issue.rb                                |  13 +-
 app/models/key.rb                                  |  10 +-
 app/models/label.rb                                |   4 +-
 app/models/label_link.rb                           |   2 +-
 app/models/lfs_object.rb                           |   4 +-
 app/models/member.rb                               |   8 +-
 app/models/members/group_member.rb                 |   8 +-
 app/models/members/project_member.rb               |   8 +-
 app/models/merge_request.rb                        |  16 +-
 app/models/merge_request_diff.rb                   |   2 +-
 app/models/milestone.rb                            |   8 +-
 app/models/namespace.rb                            |  15 +-
 app/models/note.rb                                 |  14 +-
 app/models/oauth_access_token.rb                   |   6 +-
 app/models/personal_snippet.rb                     |  10 +-
 app/models/project.rb                              |  91 +--
 app/models/project_services/asana_service.rb       |   4 +-
 app/models/project_services/assembla_service.rb    |   4 +-
 app/models/project_services/bamboo_service.rb      |   4 +-
 app/models/project_services/buildkite_service.rb   |   4 +-
 .../project_services/builds_email_service.rb       |   4 +-
 app/models/project_services/campfire_service.rb    |   4 +-
 app/models/project_services/ci_service.rb          |   4 +-
 .../custom_issue_tracker_service.rb                |   4 +-
 app/models/project_services/drone_ci_service.rb    |   4 +-
 .../project_services/emails_on_push_service.rb     |   4 +-
 .../project_services/external_wiki_service.rb      |   4 +-
 app/models/project_services/flowdock_service.rb    |   4 +-
 app/models/project_services/gemnasium_service.rb   |   4 +-
 app/models/project_services/gitlab_ci_service.rb   |   4 +-
 .../gitlab_issue_tracker_service.rb                |   4 +-
 app/models/project_services/hipchat_service.rb     |   4 +-
 app/models/project_services/irker_service.rb       |   4 +-
 .../project_services/issue_tracker_service.rb      |   4 +-
 app/models/project_services/jira_service.rb        |   4 +-
 .../project_services/pivotaltracker_service.rb     |   4 +-
 app/models/project_services/pushover_service.rb    |   4 +-
 app/models/project_services/redmine_service.rb     |   4 +-
 app/models/project_services/slack_service.rb       |   4 +-
 app/models/project_services/teamcity_service.rb    |   4 +-
 app/models/project_snippet.rb                      |  10 +-
 app/models/protected_branch.rb                     |   6 +-
 app/models/release.rb                              |   2 +-
 app/models/security_event.rb                       |   4 +-
 app/models/sent_notification.rb                    |   8 +-
 app/models/service.rb                              |   4 +-
 app/models/snippet.rb                              |  10 +-
 app/models/subscription.rb                         |   2 +-
 app/models/user.rb                                 |  62 +-
 db/schema.rb                                       | 730 ++++++++++++---------
 72 files changed, 748 insertions(+), 619 deletions(-)

This is incredibly annoying as my otherwise small diff is now huge which in turn makes the review process more annoying.

I don't really understand why we have this Gem in the first place, let alone run it automatically. Having to automatically sync comments with a database feels like over-engineering to me. On top of that the only source of database information that is always up to date (and doesn't try to normalize DB specific information) is your database itself and getting this data in turn is as simple as:

psql -U gitlab-user-here gitlab-database-here
\d table_name_here

cc @dzaporozhets @zj @rspeicher

The actual diff (to show how ridiculous this really is) can be seen here: https://gitlab.com/gitlab-org/gitlab-ce/snippets/19085.

I do like the annotations on top of the models. But in this case it does seem to be a little to eager. Maybe once a while creating a MR for it is fine too.

I would be +1 for removing it. It tends to add noise to diffs, and it creates problems when DB columns are in different order (Yes that happens sometimes).

What is wrong with opening schema.rb if you quickly want to know the fields of a table?

Yep. I'd be in favor of removing annotations (the gem and the comments).

They're handy, but if you don't do them automatically, they become out of date, and the only thing worse than no documentation is out-of-date documentation. If you do them automatically, well, see above.

Let's let the schema be the single source of truth.

Yep. I'd be in favor of removing annotations (the gem and the comments).

I agree. It makes noise if keep up to date. And if not - its useless.

Let's let the schema be the single source of truth.

@yorickpeterse I think since we all agree on removal so far - you can proceed

@yorickpeterse If you want this is something I can take care of :-)

mentioned in merge request !4099 (merged)

Status changed to closed by commit f1479b56

mentioned in commit f1479b56

mentioned in commit f96c34ec

Mentioned in commit jmcgeheeiv/gitlab-ce@f96c34ec

Mentioned in commit jwgoh/gitlab-ce@f1479b56