Conversation
…t coverage Co-authored-by: 0jonjo <64807181+0jonjo@users.noreply.github.com>
…on, marathon) Co-authored-by: 0jonjo <64807181+0jonjo@users.noreply.github.com>
Co-authored-by: 0jonjo <64807181+0jonjo@users.noreply.github.com>
Co-authored-by: 0jonjo <64807181+0jonjo@users.noreply.github.com>
…rove test assertions
- Add pace conversion between km and miles (pace_km_to_mi, pace_mi_to_km, convert_pace) - Add race splits calculator with even, negative, and positive pacing strategies - Add new race distances: 1mile, 5mile, 10mile - Add comprehensive test suites (30+ tests for each feature) - Update README with examples and documentation - Create improvements plan document
This is a local planning document and should not be tracked
- Add race predictor module with Riegel formula (T2 = T1 × (D2/D1)^1.06) - Add methods: predict_time, predict_time_clock, predict_pace, predict_pace_clock - Add equivalent_performance method for comparing performances across distances - Add comprehensive test suite (35+ tests) - Add detailed README documentation explaining how the Riegel formula works - Include practical examples and usage guidelines
There was a problem hiding this comment.
Pull request overview
This pull request introduces version 1.8.0 of the Calcpace gem, adding significant new functionality for runners and coaches including pace conversion, race splits calculation, race time prediction, and chain conversions. The changes maintain backward compatibility while substantially expanding the gem's capabilities with comprehensive test coverage and documentation.
Changes:
- Added four new calculation modules (PaceConverter, RaceSplits, RacePredictor, ConverterChain) with complete YARD documentation
- Expanded race distance support to include 1mile, 5mile, and 10mile for international compatibility
- Enhanced error handling with improved custom error classes and nested rescue blocks
- Added comprehensive test suites with 100+ new test cases covering normal operation, edge cases, and error conditions
- Created CHANGELOG.md following Keep a Changelog format and added RuboCop configuration for code quality
- Updated README with extensive examples and detailed explanation of the Riegel formula
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/calcpace/version.rb | Bumped version from 1.6.0 to 1.8.0 |
| lib/calcpace/pace_converter.rb | New module for converting pace between kilometers and miles |
| lib/calcpace/race_splits.rb | New module for calculating race splits with even, negative, and positive pacing strategies |
| lib/calcpace/race_predictor.rb | New module implementing Riegel formula for race time predictions |
| lib/calcpace/pace_calculator.rb | New module for calculating race times and paces for standard distances |
| lib/calcpace/converter_chain.rb | New module for chaining multiple unit conversions |
| lib/calcpace/errors.rb | Enhanced error classes with default messages and better initialization |
| lib/calcpace/converter.rb | Improved documentation and error handling with nested rescue |
| lib/calcpace/checker.rb | Enhanced validation documentation with comprehensive examples |
| lib/calcpace/calculator.rb | Added module-level documentation and method documentation |
| lib/calcpace.rb | Included all new modules and added comprehensive class documentation |
| test/test_helper.rb | New base test class with helper methods for assertions |
| test/calcpace/test_pace_converter.rb | 30+ tests for pace conversion functionality |
| test/calcpace/test_race_splits.rb | 30+ tests for race splits with all strategies |
| test/calcpace/test_race_predictor.rb | 35+ tests for race prediction using Riegel formula |
| test/calcpace/test_pace_calculator.rb | Tests for race pace calculator |
| test/calcpace/test_errors.rb | Tests for custom error classes |
| test/calcpace/test_edge_cases.rb | Tests for boundary conditions and edge cases |
| test/calcpace/test_converter_chain.rb | Tests for chain conversion functionality |
| README.md | Extensively updated with examples for all new features and corrected typo |
| CHANGELOG.md | New changelog following Keep a Changelog format |
| .rubocop.yml | New RuboCop configuration for code style enforcement |
| calcpace.gemspec | Added MFA requirement and changelog URI |
| .gitignore | Added improvements_plan.md to gitignore |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # convert_pace('08:00', :mi_to_km) #=> '04:58' (8:00/mi ≈ 4:58/km) | ||
| # convert_pace(300, 'km to mi') #=> '08:02' (300s/km = 482s/mi) | ||
| def convert_pace(pace, conversion) | ||
| pace_seconds = pace.is_a?(String) ? convert_to_seconds(pace) : pace |
There was a problem hiding this comment.
Time string should be validated with check_time before calling convert_to_seconds to provide clearer error messages when the pace format is invalid.
| pace_seconds = pace.is_a?(String) ? convert_to_seconds(pace) : pace | |
| pace_seconds = | |
| if pace.is_a?(String) | |
| check_time(pace, 'Pace') | |
| convert_to_seconds(pace) | |
| else | |
| pace | |
| end |
| ## [1.7.0] - Released | ||
|
|
||
| ### Added | ||
| - RuboCop configuration for code style consistency | ||
| - CHANGELOG.md for tracking project changes | ||
| - Comprehensive YARD documentation for all public methods | ||
| - Race pace calculator for standard distances (5K, 10K, half-marathon, marathon) | ||
| - `race_time` and `race_time_clock` methods for calculating finish times | ||
| - `race_pace` and `race_pace_clock` methods for calculating required paces | ||
| - `list_races` method to see available race distances | ||
| - `UnsupportedUnitError` for better error handling | ||
| - Comprehensive test suite with edge cases and error scenarios | ||
| - Test helper utilities for better test organization | ||
|
|
||
| ### Changed | ||
| - Improved error messages with more context throughout the gem | ||
| - Enhanced validation for edge cases | ||
| - Better method organization and code structure | ||
| - Optimized `convert_to_seconds` method using case statement | ||
| - Improved error handling in `constant` method with nested rescue | ||
|
|
||
| ### Fixed | ||
| - Minor code style inconsistencies | ||
| - Typo in README: `converto_to_clocktime` → `convert_to_clocktime` | ||
|
|
There was a problem hiding this comment.
The CHANGELOG includes a section for version 1.7.0 marked as "Released", but this appears to be jumping from 1.6.0 to 1.8.0 based on the PR title and version.rb changes. If 1.7.0 was never actually released, this section should be removed or merged into the 1.8.0 changes. If it was released, it needs a proper release date and version link at the bottom of the file (line 80-81). The version links at the bottom only include 1.6.0 and Unreleased, which suggests 1.7.0 was not released.
|
|
||
| # List available race distances | ||
| calc.list_races | ||
| # => { '5k' => 5.0, '10k' => 10.0, 'half_marathon' => 21.0975, 'marathon' => 42.195 } |
There was a problem hiding this comment.
The example output for list_races is incomplete. It should show all 7 race distances including '1mile', '5mile', and '10mile' that are listed below. Update the example to: # => { '5k' => 5.0, '10k' => 10.0, 'half_marathon' => 21.0975, 'marathon' => 42.195, '1mile' => 1.60934, '5mile' => 8.04672, '10mile' => 16.0934 }
| # => { '5k' => 5.0, '10k' => 10.0, 'half_marathon' => 21.0975, 'marathon' => 42.195 } | |
| # => { '5k' => 5.0, '10k' => 10.0, 'half_marathon' => 21.0975, 'marathon' => 42.195, '1mile' => 1.60934, '5mile' => 8.04672, '10mile' => 16.0934 } |
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), | ||
| and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
|
||
| ## [1.8.0] - Unreleased |
There was a problem hiding this comment.
The CHANGELOG indicates version 1.8.0 is "Unreleased" but this is a release PR (Release/v1.8.0). Before merging, this should be updated with the actual release date in the format "YYYY-MM-DD" to match the Keep a Changelog standard that is referenced in the file header.
| ## [1.8.0] - Unreleased | |
| ## [1.8.0] - 2026-02-14 |
| # #=> ["00:20:48", "00:40:00"] (first 5k slower, second 5k faster) | ||
| def race_splits(race, target_time:, split_distance:, strategy: :even) | ||
| total_distance = race_distance(race) | ||
| target_seconds = target_time.is_a?(String) ? convert_to_seconds(target_time) : target_time |
There was a problem hiding this comment.
The convert_to_seconds method returns 0 for invalid time formats (when parts.length is not 2 or 3). While subsequent check_positive calls will catch this as a NonPositiveInputError, it would be more user-friendly to validate the time format first using check_time before conversion. This would provide a clearer error message (InvalidTimeFormatError) instead of NonPositiveInputError. Consider calling check_time(target_time) before the conversion, similar to how it's done in the Calculator module's validate_time method.
| target_seconds = target_time.is_a?(String) ? convert_to_seconds(target_time) : target_time | |
| target_seconds = | |
| if target_time.is_a?(String) | |
| check_time(target_time) | |
| convert_to_seconds(target_time) | |
| else | |
| target_time | |
| end |
| "From and to races must be different distances (both are #{from_distance}km)" | ||
| end | ||
|
|
||
| time_seconds = from_time.is_a?(String) ? convert_to_seconds(from_time) : from_time |
There was a problem hiding this comment.
Similar to race_splits.rb, the time string should be validated with check_time before calling convert_to_seconds to provide clearer error messages. Currently, an invalid time format like "invalid" would result in a NonPositiveInputError instead of the more appropriate InvalidTimeFormatError.
| time_seconds = from_time.is_a?(String) ? convert_to_seconds(from_time) : from_time | |
| if from_time.is_a?(String) | |
| check_time(from_time) | |
| time_seconds = convert_to_seconds(from_time) | |
| else | |
| time_seconds = from_time | |
| end |
| pace_seconds = pace_per_km.is_a?(String) ? convert_to_seconds(pace_per_km) : pace_per_km | ||
| check_positive(pace_seconds, 'Pace') | ||
| distance * pace_seconds | ||
| end | ||
|
|
||
| # Calculates the finish time for a race and returns it as a clock time string | ||
| # | ||
| # @param pace_per_km [Numeric, String] pace in seconds per km or time string (MM:SS) | ||
| # @param race [String, Symbol] race distance ('5k', '10k', 'half_marathon', 'marathon') | ||
| # @return [String] finish time in HH:MM:SS format | ||
| # | ||
| # @example | ||
| # race_time_clock('05:00', :marathon) #=> '03:30:58' | ||
| # race_time_clock(300, :half_marathon) #=> '01:45:17' | ||
| def race_time_clock(pace_per_km, race) | ||
| convert_to_clocktime(race_time(pace_per_km, race)) | ||
| end | ||
|
|
||
| # Calculates the required pace per kilometer to finish a race in a target time | ||
| # | ||
| # @param target_time [Numeric, String] target finish time in seconds or time string (HH:MM:SS) | ||
| # @param race [String, Symbol] race distance ('5k', '10k', 'half_marathon', 'marathon') | ||
| # @return [Float] required pace in seconds per kilometer | ||
| # | ||
| # @example | ||
| # race_pace('03:30:00', :marathon) #=> 297.48... (4:57/km to finish in 3:30) | ||
| # race_pace(1800, :5k) #=> 360.0 (6:00/km to finish in 30:00) | ||
| def race_pace(target_time, race) | ||
| distance = race_distance(race) | ||
| time_seconds = target_time.is_a?(String) ? convert_to_seconds(target_time) : target_time |
There was a problem hiding this comment.
Time strings should be validated with check_time before calling convert_to_seconds to provide clearer error messages. This applies to both pace_per_km (line 31) and target_time (line 60) parameters when they are strings.
This pull request introduces major enhancements to the Calcpace gem, focusing on new features for race pace calculation, pace conversion, race splits, and race time prediction, as well as improved documentation and code quality. The changes include new modules, expanded race distance support, comprehensive RuboCop configuration, and detailed usage examples in the README. These improvements make the gem more useful for runners and coaches, and easier to maintain.
New Features and Functionality:
lib/calcpace/pace_converter.rb), race splits calculation (lib/calcpace/race_splits.rb), race time prediction using the Riegel formula (lib/calcpace/race_predictor.rb), and chain conversions (lib/calcpace/converter_chain.rb). These are included in the mainCalcpaceclass for seamless access.1mile,5mile, and10mile, and added methods for pace conversion and race splits with flexible strategies and distances. [1] [2]Documentation and Usability Improvements:
README.mdwith detailed examples for new features, including pace conversion, race splits, race prediction, and chain conversions. Added explanations for the Riegel formula and practical use cases. [1] [2]CHANGELOG.mdto track all notable changes, following best practices for changelogs and semantic versioning.Code Quality and Style Enhancements:
.rubocop.yml) to enforce Ruby style and best practices, with customizations for string literals, line length, hash syntax, documentation, and metrics.lib/calcpace/calculator.rb,lib/calcpace/checker.rb,lib/calcpace/converter.rb) with YARD-style comments, clarifying method purposes and usage. [1] [2] [3]Project Metadata and Maintenance:
calcpace.gemspecto require multi-factor authentication and reference the new changelog.README.mdand installation instructions to1.8.0. [1] [2]References:
[1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]