-
Notifications
You must be signed in to change notification settings - Fork 661
Rails 8 support #1046
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
Rails 8 support #1046
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @groach, 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!
I've updated the project to support Rails 8, which involved a comprehensive upgrade of the Ruby version and various gem dependencies. This ensures the application remains compatible with the latest Ruby and Rails ecosystems. I also made several code adjustments to align with modern Ruby conventions and improve overall code quality, including updates to Rubocop configurations and internal method naming.
Highlights
- Ruby Version Upgrade: The project's Ruby version requirement has been significantly updated across
.tool-versions
,Gemfile
, andannotate.gemspec
to support Ruby 3.0 and above, with the.tool-versions
file now specifying Ruby 3.4.4. - Rails and Gem Dependency Updates: The
activerecord
gem dependency has been broadened to include Rails 8 and potentially Rails 9, reflecting forward compatibility efforts. Additionally, Rubocop and Rubocop-RSpec gems have been updated to their latest compatible versions, along with disabling some new cops to maintain current code style. - Ruby 3.x Compatibility Adjustments: Several code adjustments were made to ensure compatibility with newer Ruby versions, including the adoption of the safe navigation operator (
&.
), removal of explicitRegexp.new
and.freeze
for regex literals, and updates to array processing methods likemap
toto_h
andsize.zero?
toempty?
. - Method Renaming for Ruby Conventions: Key internal methods like
annotate_one_file
andremove_annotation_of_file
inlib/annotate/annotate_models.rb
have been renamed with a?
suffix, indicating they return a boolean value, aligning with Ruby's convention for predicate methods. Corresponding test files have been updated to reflect these changes.
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 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 or fill out our survey 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.
Footnotes
-
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. ↩
There was a problem hiding this 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 brings support for Rails 8 by updating the Ruby version and gem dependencies. It includes many great refactorings that modernize the codebase, improve code style, and enhance test efficiency. I've found a couple of areas for potential improvement. One is a potential issue with load order consistency due to the removal of .sort
, and another is a minor suggestion for code clarity. Overall, this is a solid update.
@@ -89,7 +89,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| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of .sort
here might introduce non-deterministic load order for files. Dir.glob
doesn't guarantee any specific order for the files returned; it's dependent on the filesystem. By sorting the files, the previous implementation ensured a consistent load order, which can be crucial to prevent issues related to load order dependencies (e.g., uninitialized constant
errors). If the order does not matter, this is fine, but if it does, it would be safer to reintroduce .sort
.
Dir.glob("#{load_path}/**/*.rb").sort.each do |file|
out = magic_comments_map.map do |magic_comment| | ||
magic_comment | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.