Skip to content

V1.0.0 #1043

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

Closed
wants to merge 14 commits into from
Closed

V1.0.0 #1043

wants to merge 14 commits into from

Conversation

chemica
Copy link

@chemica chemica commented Jun 3, 2025

No description provided.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @chemica, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request represents a significant update to the project, effectively forking the original annotate gem to provide compatibility with modern Ruby (specifically 3.2.2+) and Rails (8.0.0+). The changes encompass updating core dependencies, modernizing the codebase to align with newer Ruby features and best practices, and cleaning up code style through extensive RuboCop configuration updates. The gem name has also been changed to annot8 to reflect its status as a fork aimed at supporting these newer versions.

Highlights

  • Gem Fork & Renaming: The project is now a fork of the original annotate gem and has been renamed to annot8.
  • Dependency Updates: Updated activerecord requirement to support Rails 8.0.0+ and set the required Ruby version to 3.2.2+. Added json, ruby-lsp, and racc gems. Removed version constraints from rubocop and rubocop-rspec to allow using the latest versions.
  • RuboCop Modernization: Updated RuboCop configuration (.rubocop.yml, .rubocop_todo.yml), enabling many previously disabled cops and enforcing stricter code style, including enabling Style/FrozenStringLiteralComment and updating target Ruby version.
  • Ruby 3.2+ Compatibility: Addressed compatibility issues with Ruby 3.2+, such as replacing File.exists? with File.exist? (via a patch), using __dir__ instead of File.dirname(__FILE__), and using ENV.fetch for accessing environment variables safely.
  • Code Style Improvements: Replaced usages of $stderr.puts with warn and updated various string formatting (sprintf to format) and regex literal usages (/.../ to %r{...}) for better style and compatibility.

Changelog

Click here to see the changelog
  • .gitignore
    • Removed .ruby-* and /Gemfile.lock from ignored files.
  • .rubocop.yml
    • Changed require to plugins for RuboCop extensions.
    • Added TargetRubyVersion: 3.2.2 to AllCops.
    • Enabled Style/FrozenStringLiteralComment.
  • .rubocop_todo.yml
    • Significantly reduced the number of excluded/disabled cops, indicating code style improvements.
    • Updated the RuboCop version comment to 1.75.8.
    • Adjusted configurations for various cops (e.g., Layout/ExtraSpacing, Metrics/AbcSize, Metrics/MethodLength, RSpec/ExampleLength, RSpec/NestedGroups).
  • .ruby-version
    • Added Ruby version 3.2.2.
  • Gemfile
    • Updated activerecord dependency to >= 8.0.0, < 9.
    • Added json, racc, and ruby-lsp gems.
    • Removed version constraints from rubocop and rubocop-rspec.
    • Removed ruby_dep gem.
    • Added frozen_string_literal: true comment.
  • Gemfile.lock
    • Added a new Gemfile.lock file reflecting the updated dependencies for Ruby 3.2.2 and Rails 8.0.2.
  • Guardfile
    • Added frozen_string_literal: true comment.
    • Added all_on_start: true option to the rspec guard.
    • Changed some block syntax to use do...end.
  • README.md
    • Added a note indicating this is a fork for Rails 8 and Ruby 3.2.2+.
    • Changed gem name references from annotate to annot8.
    • Updated repository URL references from ctran/annotate_models to chemica/annotate_models.
  • Rakefile
    • Added frozen_string_literal: true comment.
    • Replaced $stderr.puts with warn.
    • Updated regex delimiters from /.../ to %r{...} for path matching.
    • Slightly adjusted the logic for adding runtime dependencies in the gem:gemspec task.
  • annotate.gemspec
    • Changed the gem name from annotate to annot8.
    • Updated the required Ruby version to >= 3.2.2.
    • Updated authors and email information.
    • Updated the description to mention the fork and compatibility with Rails 8 and Ruby 3.2.2+.
    • Updated homepage and source code URIs to the chemica repository.
    • Updated activerecord dependency to >= 8.0.0, < 9.
    • Added rubygems_mfa_required metadata.
  • bin/annotate
    • Added frozen_string_literal: true comment.
    • Simplified the abort condition.
    • Replaced File.expand_path(File.dirname __FILE__) with __dir__.
    • Changed ENV['is_rake'] access to ENV.fetch('is_rake', nil).
  • lib/annotate.rb
    • Added frozen_string_literal: true comment.
    • Added rubocop:disable Metrics/ModuleLength comment.
    • Changed ENV[...] access to ENV.fetch(..., nil).
    • Simplified conditional assignments for options.
    • Removed .sort from Dir.glob iteration.
    • Changed bare rescue to rescue StandardError.
    • Replaced $stderr.puts with warn.
  • lib/annotate/active_record_patch.rb
    • Added frozen_string_literal: true comment.
  • lib/annotate/annotate_models.rb
    • Added frozen_string_literal: true comment.
    • Removed .freeze from constants.
    • Changed MATCHED_TYPES, NO_LIMIT_COL_TYPES, NO_DEFAULT_COL_TYPES to use array literals directly.
    • Changed MAGIC_COMMENT_MATCHER to use a literal regex.
    • Added skip_subdirectory_model_load as an attr_writer.
    • Updated regex delimiter from /.../ to %r{...}.
    • Used String.new('') for creating mutable strings.
    • Used format instead of sprintf for string formatting.
    • Used to_h for hash conversion.
    • Updated get_attributes logic for primary key and simple indexes.
    • Changed bare rescue to rescue StandardError.
    • Replaced $stderr.puts with warn.
    • Simplified conditional assignments.
    • Adjusted array slicing in get_model_class.
  • lib/annotate/annotate_models/file_patterns.rb
    • Added frozen_string_literal: true comment.
  • lib/annotate/annotate_routes.rb
    • Added frozen_string_literal: true comment.
    • Changed require_relative paths to use relative paths without ./.
    • Changed remove_annotations method signature.
    • Changed header = header << '' to header <<= ''.
  • lib/annotate/annotate_routes/header_generator.rb
    • Added frozen_string_literal: true comment.
    • Removed .freeze from constants.
    • Changed require_relative path.
    • Changed rake routes split delimiter.
    • Changed regex delimiter from /.../ to %r{...}.
    • Changed array slice [1..-1] to [1..].
    • Used format instead of sprintf.
  • lib/annotate/annotate_routes/helpers.rb
    • Added frozen_string_literal: true comment.
    • Removed .freeze from constant.
    • Changed split delimiter.
    • Simplified return value in real_content_and_header_position.
  • lib/annotate/constants.rb
    • Added frozen_string_literal: true comment.
    • Removed .freeze from constant regex.
    • Simplified array literals.
  • lib/annotate/helpers.rb
    • Added frozen_string_literal: true comment.
    • Changed ENV[...] access to ENV.fetch(..., nil).
  • lib/annotate/parser.rb
    • Added frozen_string_literal: true comment.
    • Removed Metrics/ClassLength disable comment.
    • Simplified array literal.
  • lib/annotate/tasks.rb
    • Added frozen_string_literal: true comment.
  • lib/annotate/version.rb
    • Added frozen_string_literal: true comment.
    • Updated version to '1.0.0'.
  • lib/generators/annotate/install_generator.rb
    • Added frozen_string_literal: true comment.
  • lib/generators/annotate/templates/auto_annotate_models.rake
    • Added frozen_string_literal: true comment.
    • Updated default options hash syntax.
  • lib/tasks/annotate_models.rake
    • Added frozen_string_literal: true comment.
    • Changed File.expand_path(File.dirname(File.dirname(__FILE__))) to File.expand_path(File.dirname(__FILE__, 2)).
    • Changed ENV[...] access to ENV.fetch(..., nil).
    • Updated default options hash syntax.
  • lib/tasks/annotate_models_migrate.rake
    • Added frozen_string_literal: true comment.
    • Changed array literal.
    • Slightly adjusted the logic in update_annotations.
  • lib/tasks/annotate_routes.rake
    • Added frozen_string_literal: true comment.
    • Changed File.expand_path(File.dirname(File.dirname(__FILE__))) to File.expand_path(File.dirname(__FILE__, 2)).
    • Changed ENV[...] access to ENV.fetch(..., nil).
  • spec/lib/annotate/annotate_models/file_patterns_spec.rb
    • Added frozen_string_literal: true comment.
    • Changed subject to let.
    • Updated expectations to use the new let variable.
  • spec/lib/annotate/annotate_models_spec.rb
    • Added frozen_string_literal: true comment.
    • Changed double to instance_double where appropriate for better mocking.
    • Updated mock methods to align with instance_double usage.
    • Changed subject to let in various contexts.
    • Updated expectations to use the new let variables.
    • Changed before :each to before and after :each to after.
    • Changed expect(...).to_not receive to expect(...).not_to receive.
    • Updated tests for the frozen option to correctly raise SystemExit with the expected message.
    • Updated get_model_class tests to reflect changes in subdirectory loading logic.
  • spec/lib/annotate/annotate_routes_spec.rb
    • Added frozen_string_literal: true comment.
    • Removed .freeze from constants.
    • Changed before :each to before.
    • Adjusted markdown header spacing in expected output.
  • spec/lib/annotate/helpers_spec.rb
    • Added frozen_string_literal: true comment.
    • Changed allow(ENV).to receive(:[]) to allow(ENV).to receive(:fetch) for accessing environment variables in tests.
  • spec/lib/annotate/parser_spec.rb
    • Added frozen_string_literal: true comment.
    • Changed before(:example) to before.
    • Changed ENV[...] access to ENV.fetch(..., nil) in expectations.
  • spec/lib/annotate_spec.rb
    • Added frozen_string_literal: true comment.
  • spec/lib/tasks/annotate_models_migrate_spec.rb
    • Added frozen_string_literal: true comment.
    • Changed array literal.
  • spec/lib/tasks/annotate_models_spec.rb
    • Added frozen_string_literal: true comment.
    • Removed RSpec/DescribeClass disable comment.
  • spec/spec_helper.rb
    • Added frozen_string_literal: true comment.
    • Required support/file_exists_patch.
  • spec/support/file_exists_patch.rb
    • Added a new file to monkey patch File.exists? for compatibility with Ruby 3.2+.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Old gems gather dust,
New versions bring features bright,
Codebase shines anew.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a significant version bump to V1.0.0, likely reflecting the substantial updates for Rails 8 and Ruby 3.2.2+ compatibility. The changes include updates to dependencies, RuboCop configuration, and various code style improvements across the codebase, such as adopting frozen string literals and more modern Ruby syntax.

Overall, the updates are positive and move the project towards better practices and compatibility with newer environments. However, there are a couple of areas that warrant further discussion to ensure code quality and prevent potential issues.

Summary of Findings

  • Broad RSpec Exclusions in RuboCop TODO: Several RSpec-related RuboCop cops have been configured with broad spec/**/* exclusions in .rubocop_todo.yml. This might mask potential issues in the test suite and could affect long-term test maintainability.
  • Removal of Sort in Eager Loading Path: In lib/annotate.rb, the .sort method was removed from a Dir.glob call that processes eager_load_paths for older Rails versions. This could potentially alter file loading order, which might be significant if there are inter-dependencies sensitive to load order.

Merge Readiness

This pull request brings important updates for compatibility with modern Rails and Ruby versions. The code is largely improved with style updates and dependency management.

However, there are two medium-severity concerns identified:

  1. The broad exclusions for RSpec cops in the RuboCop configuration might hinder test quality and maintainability.
  2. The removal of sorting when processing eager load paths could potentially affect file loading order in older Rails applications.

I recommend addressing these points before merging to ensure the stability and quality of the codebase. As an AI, I am not authorized to approve pull requests; please ensure further review and approval from other maintainers.

Comment on lines +143 to 190

# Offense count: 10
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: implicit, each, example
RSpec/HookArgument:
RSpec/ExpectInHook:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'
- 'spec/lib/annotate/annotate_routes_spec.rb'
- 'spec/lib/annotate/parser_spec.rb'

# Offense count: 1
# Cop supports --auto-correct.
RSpec/HooksBeforeExamples:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'

# Offense count: 78
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: single_line_only, single_statement_only, disallow
RSpec/ImplicitSubject:
Exclude:
- 'spec/lib/annotate/annotate_models/file_patterns_spec.rb'
- 'spec/lib/annotate/annotate_models_spec.rb'
- 'spec/lib/annotate/helpers_spec.rb'

# Offense count: 51
# Configuration parameters: AssignmentOnly.
RSpec/InstanceVariable:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'

# Offense count: 4
# Cop supports --auto-correct.
RSpec/LeadingSubject:
# Configuration parameters: Max, AllowedIdentifiers, AllowedPatterns.
RSpec/IndexedLet:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'
- 'spec/lib/annotate/helpers_spec.rb'

# Offense count: 9
# Offense count: 6
RSpec/LeakyConstantDeclaration:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'
- 'spec/lib/annotate/annotate_routes_spec.rb'

# Offense count: 108
# Offense count: 109
# Configuration parameters: EnforcedStyle.
# SupportedStyles: have_received, receive
RSpec/MessageSpies:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'
- 'spec/lib/annotate/annotate_routes_spec.rb'
- 'spec/lib/annotate/helpers_spec.rb'
- 'spec/lib/annotate/parser_spec.rb'
- 'spec/lib/tasks/annotate_models_migrate_spec.rb'

# Offense count: 36
# Offense count: 29
RSpec/MultipleExpectations:
Max: 4

# Offense count: 66
# Configuration parameters: AllowSubject.
RSpec/MultipleMemoizedHelpers:
Max: 9

# Offense count: 6
# Configuration parameters: IgnoreSharedExamples.
# Offense count: 5
# Configuration parameters: EnforcedStyle, IgnoreSharedExamples.
# SupportedStyles: always, named_only
RSpec/NamedSubject:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'
- 'spec/lib/annotate/helpers_spec.rb'
- 'spec/lib/tasks/annotate_models_migrate_spec.rb'
- 'spec/**/*'

# Offense count: 140
# Offense count: 39
# Configuration parameters: AllowedGroups.
RSpec/NestedGroups:
Max: 9

# Offense count: 2
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: not_to, to_not
RSpec/NotToNot:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'
- 'spec/lib/annotate/helpers_spec.rb'
- 'spec/**/*'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It seems several RSpec cops (RSpec/ExampleLength, RSpec/NamedSubject, RSpec/NestedGroups, RSpec/MultipleMemoizedHelpers, RSpec/InstanceVariable) now have very broad Exclude patterns like spec/**/*.

While this silences RuboCop warnings, could such broad exclusions potentially hide issues in the test suite or lead to a decline in test code quality over time? Perhaps more targeted exclusions or addressing the underlying RSpec offenses would be beneficial in the long run.

@@ -89,7 +96,7 @@ def self.eager_load(options)
if Rails.version.split('.').first.to_i < 3
Rails.configuration.eager_load_paths.each do |load_path|
matcher = /\A#{Regexp.escape(load_path)}(.*)\.rb\Z/
Dir.glob("#{load_path}/**/*.rb").sort.each do |file|
Dir.glob("#{load_path}/**/*.rb").each do |file|

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The .sort call was removed from Dir.glob("#{load_path}/**/*.rb").sort.each during the eager loading process.

Could the order of loading files in Rails.configuration.eager_load_paths be significant, especially for older Rails versions (Rails < 3 as per the conditional)? If there are implicit dependencies based on load order, removing .sort might introduce subtle bugs or change behavior. Was this change tested for potential load order issues?

@chemica chemica closed this Jun 3, 2025
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.

1 participant