Skip to content
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

add method that generate roman numbers up to 100 #462

Merged
merged 2 commits into from
Dec 3, 2021

Conversation

wilkus27
Copy link
Contributor

@wilkus27 wilkus27 commented Dec 2, 2021

Done for Intellectual Property. This book has over 80 footnotes in roman numbers, so I added method that generate them rather then create bigger constant. I had problems with instance variable. Adding attr_reader or attr_accesor at the beginning of the class didn't help, methods from this class treated variable as nil. I found another solution, but if someone have better idea how to handle this let me know 😉

@wilkus27 wilkus27 self-assigned this Dec 2, 2021
@codecov
Copy link

codecov bot commented Dec 2, 2021

Codecov Report

Merging #462 (e4d3421) into main (be48e67) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #462   +/-   ##
=======================================
  Coverage   99.96%   99.96%           
=======================================
  Files         186      186           
  Lines        2818     2829   +11     
=======================================
+ Hits         2817     2828   +11     
  Misses          1        1           
Impacted Files Coverage Δ
lib/kitchen/patches/integer.rb 100.00% <100.00%> (ø)

Copy link
Contributor

@LostMojo LostMojo left a comment

Choose a reason for hiding this comment

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

It looks good to me! I'd maybe add a spec to test if it gets 9s correctly, but otherwise, it looks pretty nice!

@wilkus27 wilkus27 merged commit 8100647 into main Dec 3, 2021
@wilkus27 wilkus27 deleted the more-roman-footnotes branch December 3, 2021 09:52
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.

2 participants