trouble with annotate after upgrading from 2.7.5 to 3.0.2

[gingerlime]

I'm trying to upgrade from 2.7.5 to 3.0.2 but somehow it looks like annotate fails silently and doesn't work any more...

A couple of things:

• our CI pipeline runs bundle exec annotate to make sure no model annotations are missing. This now fails. Perhaps annotate no longer outputs "Model files unchanged" when there are no changes? I'm not sure
• I tried to create a simple migration to a table and running rake db:migrate, but it doesn't automatically add annotations (no errors, I did add the rake task using rails g annotate:install).
• also manually running bundle exec annotate after the migration was run (I can see schema.rb updated), doesn't update the annotations... It just doesn't seem to do anything... No errors or exit code other than 0 either.

Commands

$ git st
On branch dependabot/bundler/annotate-3.0.2
Your branch is up to date with 'origin/dependabot/bundler/annotate-3.0.2'.

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

        new file:   db/migrate/20190930085602_blablabla.rb
        new file:   lib/tasks/auto_annotate_models.rake

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   db/schema.rb

$ git diff db/schema.rb
diff --git a/db/schema.rb b/db/schema.rb
index 4ddfcf5e8..3d1173ce6 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.

-ActiveRecord::Schema.define(version: 2019_08_22_094809) do
+ActiveRecord::Schema.define(version: 2019_09_30_085602) do

   # These are extensions that must be enabled in order to support this database
   enable_extension "hstore"
@@ -469,6 +469,7 @@ ActiveRecord::Schema.define(version: 2019_08_22_094809) do
     t.string "active_session_id"
+    t.boolean "xyz"

...

$ bundle exec annotate
warning: parser/current is loading parser/ruby25, which recognizes
warning: 2.5.6-compliant syntax, but you are running 2.5.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
2019-09-30 09:07:40.448472 DEBUG name=Sidekiq message=Sidekiq client with redis options {:url=>"redis://redis:6379", :reconnect_attempts=>10, :reconnect_delay=>0.3, :reconnect_delay_max=>9, :id=>"Sidekiq-client-PID-1211"} payload=null tags= named_tags= duration= process_info=1211:47101335709180 exception=

Version

• annotate version (3.0.2)
• rails version (5.2)
• ruby version (2.5.3)

[nesrual]

+1

Same problem here using 3.0.2

[drwl]

@gingerlime thanks for reporting. I appreciate the time and detail in the issue. There were some changes to the way model annotations were done in v3.0.0, so it's possible it's due to that.

Would it be possible for you to provide an example repository for me to debug with?

[drwl]

Also would you mind posting your generated auto_annotate_models.rake if you have one? I have a guess of what might be happening.

[gingerlime]

@drwl here's the rake task... It looks like models are set to false for some strange reason

$ cat lib/tasks/auto_annotate_models.rake

# NOTE: only doing this in development as some production environments (Heroku)
# NOTE: are sensitive to local FS writes, and besides -- it's just not proper
# NOTE: to have a dev-mode tool do its thing in production.
if Rails.env.development?
  require 'annotate'
  task :set_annotation_options do
    # You can override any of these by setting an environment variable of the
    # same name.
    Annotate.set_defaults(
      'additional_file_patterns'    => [],
      'routes'                      => 'false',
      'models'                      => 'false',
      'position_in_routes'          => 'before',
      'position_in_class'           => 'before',
      'position_in_test'            => 'before',
      'position_in_fixture'         => 'before',
      'position_in_factory'         => 'before',
      'position_in_serializer'      => 'before',
      'show_foreign_keys'           => 'true',
      'show_complete_foreign_keys'  => 'false',
      'show_indexes'                => 'true',
      'simple_indexes'              => 'false',
      'model_dir'                   => 'app/models',
      'root_dir'                    => '',
      'include_version'             => 'false',
      'require'                     => '',
      'exclude_tests'               => 'false',
      'exclude_fixtures'            => 'false',
      'exclude_factories'           => 'false',
      'exclude_serializers'         => 'false',
      'exclude_scaffolds'           => 'true',
      'exclude_controllers'         => 'true',
      'exclude_helpers'             => 'true',
      'exclude_sti_subclasses'      => 'false',
      'ignore_model_sub_dir'        => 'false',
      'ignore_columns'              => nil,
      'ignore_routes'               => nil,
      'ignore_unknown_models'       => 'false',
      'hide_limit_column_types'     => 'integer,bigint,boolean',
      'hide_default_column_types'   => 'json,jsonb,hstore',
      'skip_on_db_migrate'          => 'false',
      'format_bare'                 => 'true',
      'format_rdoc'                 => 'false',
      'format_markdown'             => 'false',
      'sort'                        => 'false',
      'force'                       => 'false',
      'frozen'                      => 'false',
      'classified_sort'             => 'true',
      'trace'                       => 'false',
      'wrapper_open'                => nil,
      'wrapper_close'               => nil,
      'with_comment'                => 'true'
    )
  end

  Annotate.load_tasks
end

[gingerlime]

looks like changing it to 'true' fixes the problem :) shouldn't this be the default?

(also wondering, why is this 'false' (string) rather than false (boolean)?)

[kapso]

Same here, switched to 2.7.5, and now it works.

[bartoszkopinski]

Looks like the models attribute now needs to be present and explicitly set to 'true', otherwise annotate doesn't work. I didn't have this attribute added to my auto_annotate_models.rake when it was created (but that was a while ago).

[drwl]

@gingerlime @kapso @bartoszkopinski @nesrual do yall think a post install message would be helpful? Or any thoughts on ways to make it easier for folks to upgrade from v2 to v3?

[gingerlime]

@drwl can you explain why annotating models isn't true by default? At least for me, I started using the gem when it was called annotate_models, so based on the name alone, its primary and obvious purpose was to annotate models.

I can understand that now the gem branches off to other types of annotations, and doesn't want to force users into any specific choices though.

Perhaps the generator can detect if there are existing annotations and set defaults accordingly? or even default to annotate everything? (removing some types of unwanted annotations is still possible). Or adding an upgrade generator that defaults to models being true (I believe in 2.x these were the only annotations? or were there more already?)

It just feels awkward especially upgrading that it doesn't annotate my models by default.

Just my 2 cents :)

[drwl]

Sorry for the delay, had been on vacation and had some downtime. Now that I'm back I'm catching up on everything.

@gingerlime there's not really any reason. Just happens to be a regression (?) from work done in #647. I don't have strong feelings for either approach. I'm favoring an upgrade generator as it would make this less opinionated and seems to be a simple path forward.

Happy to take any PRs though for other approaches.

[TedTran2019]

Ohh, I thought it was a bug but it was just a feature.

I actually agree with gingerlime, it's really weird the gem's defaults are set to "do not annotate", when most people use the gem specifically to annotate their models and routes. Maybe continue with this same approach but set the default to 'true' instead of 'false'?

[qortex]

Have been running into the same issue (silently as annotate updated itself to 3.0.2 after a bundle update).
I second the fact that it would be more friendly if an update guide/script should enforce the previously working behavior.

I did a new rails g annotate:install overwriting my previous config and tailored the settings from there. It works fine now. Thanks!

[drwl]

@TedTran2019 @MicMicMon I cut 3.0.3 and pushed it to Rubygems. It includes #671 so any new rails g annotate:install should produce a config that defaults to annotating.

[gingerlime]

Thanks @drwl. That's definitely better. I still wonder about the upgrade process though, because when I just upgrade the gem, then running annotate simply fails silently. It only works again after running rails g annotate:install ... I would still say it's not the best upgrade experience. Not complaining, but I think others can stumble over it just like I did...

[drwl]

@gingerlime yeah that makes sense. I'll add a note to the README for now. I'll tag you in the proposed changes if that's alright with you.

[gingerlime]

Sure. 👍 Thank you @drwl !

[KelseyDH]

Weird, my generated version of auto_annotate_models.rake didn't even have a models option created that I could set to true... so to fix this breaking update I had to manually add:

'models' => 'true'

So I suspect this version update broke the builds of many projects silently who expected their migrations to still be annotated.

[drwl]

@KelseyDH yes you're right — this was a regression from previous work done. I'll have some time in next week or so during (American) holidays to add it. I was hoping someone would have been able to take it on in the mean time.