Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated Bolognese gem to support Ruby 3.2–4.0, modernized dependencies (activesupport, citeproc-ruby, csl-styles, concurrent-ruby, vcr, webmock), removed postrank-uri dependency, refactored URI normalization logic, bumped version to 2.6.0, and updated test expectations for citation output. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Gemfile (1)
3-3: Pin the git dependency to an immutable ref.
Line 3 tracks a moving branch, which makes builds non-reproducible and brittle if the branch is force-pushed or deleted.Suggested change
-gem 'maremma', '~> 5.0', git: 'https://github.com/datacite/maremma.git', branch: 'ks-ruby-4-upgrade' +gem 'maremma', '~> 5.0', git: 'https://github.com/datacite/maremma.git', ref: '<commit-sha>'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gemfile` at line 3, The Gemfile currently pins maremma to a moving branch via the gem declaration gem 'maremma', '~> 5.0', git: 'https://github.com/datacite/maremma.git', branch: 'ks-ruby-4-upgrade'; update this to pin to an immutable ref by replacing the branch: 'ks-ruby-4-upgrade' with a ref: '<COMMIT_SHA>' (or a tagged version) so the gem line references a specific commit SHA, ensuring reproducible builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bolognese.gemspec`:
- Line 16: Update the gemspec's Ruby version constraint to allow the full 4.0
minor series by replacing the upper bound on s.required_ruby_version;
specifically change the current constraint that uses "<= 4.0.1" to use "< 4.1"
so s.required_ruby_version becomes ['>= 3.2', '< 4.1'], ensuring patch releases
in 4.0.x are permitted while still preventing 4.1+ upgrades.
- Line 19: Restore the maremma runtime dependency in the gemspec by re-adding
the add_dependency entry for maremma (with an appropriate version constraint,
e.g., ~> 5.0) and ensure it is not commented out so the gemspec declares maremma
as a runtime dependency; this will allow the require in lib/bolognese.rb (line
with require 'maremma') and all usages in utils.rb, datacite_reader.rb,
npm_reader.rb, schema_org_reader.rb, etc., to work when the gem is installed
from RubyGems.
---
Nitpick comments:
In `@Gemfile`:
- Line 3: The Gemfile currently pins maremma to a moving branch via the gem
declaration gem 'maremma', '~> 5.0', git:
'https://github.com/datacite/maremma.git', branch: 'ks-ruby-4-upgrade'; update
this to pin to an immutable ref by replacing the branch: 'ks-ruby-4-upgrade'
with a ref: '<COMMIT_SHA>' (or a tagged version) so the gem line references a
specific commit SHA, ensuring reproducible builds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3070d29f-40a0-4542-8c41-6c397f0da86f
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/ci.yml.github/workflows/release.ymlGemfilebolognese.gemspeclib/bolognese/version.rb
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bolognese.gemspec`:
- Around line 29-30: The gemspec dependency bump to citeproc-ruby (~> 2.1, >=
2.1.8) and csl-styles (~> 2.0, >= 2.0.2) may pull in CSL 2.x breaking changes;
inspect any code that interacts with CSL APIs (search for uses of citeproc-ruby
rendering code and places that build CSL items/variables or validate CSL
documents), update handling for new item types (e.g., software, dataset,
standard, periodical), variable/license defaults, and schema expectations, then
run the full test suite and fix failing examples/formatting outputs;
specifically review code paths that construct CSL items or call citeproc-ruby
render methods and adjust mappings/validation so output remains consistent
before finalizing the dependency upgrade.
In `@lib/bolognese/utils.rb`:
- Around line 671-673: The current code strips a trailing slash from the
serialized URL (variable normalized_url) which can remove slashes in query or
fragment; instead, after calling uri.normalize, modify only the URI path (e.g.
remove a trailing '/' from uri.path) and then serialize to string so query and
fragment are preserved; update the block using uri.normalize and normalized_url
to mutate uri.path (not normalized_url) before calling to_s—this will fix
downstream consumers like normalize_id and calls into Maremma.get in
npm_reader.rb and schema_org_reader.rb.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b503d6c-5d79-46e5-99b6-71e15885e44d
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
bolognese.gemspeclib/bolognese.rblib/bolognese/author_utils.rblib/bolognese/metadata_utils.rblib/bolognese/readers/datacite_reader.rblib/bolognese/utils.rbspec/writers/citation_writer_spec.rb
💤 Files with no reviewable changes (3)
- lib/bolognese/metadata_utils.rb
- lib/bolognese.rb
- lib/bolognese/author_utils.rb
✅ Files skipped from review due to trivial changes (1)
- lib/bolognese/readers/datacite_reader.rb
There was a problem hiding this comment.
♻️ Duplicate comments (1)
bolognese.gemspec (1)
19-19:⚠️ Potential issue | 🔴 CriticalRe-enable
maremmain the gemspec.The gem still loads
maremmaimmediately inlib/bolognese.rb:5, and core paths useMaremma.from_json,Maremma.from_xml, andMaremma.getinlib/bolognese/utils.rb:572-584,lib/bolognese/readers/datacite_json_reader.rb:10, andlib/bolognese/readers/crossref_reader.rb:18. Leaving this dependency commented out means the published gem can install without a library it requires at load time.Proposed fix
- # s.add_dependency 'maremma', '~> 5.0' # update this to the latest version when available on RubyGems + s.add_dependency 'maremma', '~> 5.0'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bolognese.gemspec` at line 19, Uncomment and re-add the maremma runtime dependency in the gemspec so the gem installs the library it requires at load time: restore the s.add_dependency 'maremma', '~> 5.0' (or update to the latest compatible version) so require 'maremma' in lib/bolognese.rb and calls to Maremma.from_json / Maremma.from_xml / Maremma.get used in lib/bolognese/utils.rb, lib/bolognese/readers/datacite_json_reader.rb, and lib/bolognese/readers/crossref_reader.rb will be present when the gem is installed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@bolognese.gemspec`:
- Line 19: Uncomment and re-add the maremma runtime dependency in the gemspec so
the gem installs the library it requires at load time: restore the
s.add_dependency 'maremma', '~> 5.0' (or update to the latest compatible
version) so require 'maremma' in lib/bolognese.rb and calls to Maremma.from_json
/ Maremma.from_xml / Maremma.get used in lib/bolognese/utils.rb,
lib/bolognese/readers/datacite_json_reader.rb, and
lib/bolognese/readers/crossref_reader.rb will be present when the gem is
installed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efd60d4e-4bf4-4af7-85da-cda4ea58270c
📒 Files selected for processing (2)
bolognese.gemspeclib/bolognese/utils.rb
Purpose
closes: Add github issue that originated this PR
Approach
Open Questions and Pre-Merge TODOs
Learning
Types of changes
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Reviewer, please remember our guidelines:
Summary by CodeRabbit
Release Notes v2.6.0
New Features
Chores