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

Cleanup Excelx #215

Merged
merged 4 commits into from
May 28, 2015
Merged

Cleanup Excelx #215

merged 4 commits into from
May 28, 2015

Conversation

stevendaniels
Copy link
Contributor

No description provided.

+ Autoload Excelx::Cell and Excelx::Sheet
+ Added excelx/cell.rb
+ Added exelx/sheet.rb
+ Used Regexp capture match groups instead of multiple `split` methods
+ added private `create_date` method
+ renamed private methods
@coveralls
Copy link

Coverage Status

Coverage increased (+0.15%) to 93.86% when pulling e9a8894 on stevendaniels:cleanup_excelx into b2da654 on roo-rb:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.15%) to 93.86% when pulling e9a8894 on stevendaniels:cleanup_excelx into b2da654 on roo-rb:master.

:datetime
module Roo
class Excelx < Roo::Base
autoload :Workbook, 'roo/excelx/workbook'
Copy link
Member

Choose a reason for hiding this comment

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

I think we can require this modules except of autoload. No need to make autoload there.

@stevendaniels
Copy link
Contributor Author

@simonoff Thanks. I'll work on those changes.

+ require Excelx's submodules instead of autoloading.
+  Use guard class in Excelx::Comments#extract_comments and
Excelx::Relationships.extract_relationships
+ Updated conditionals in Excelx::Cell.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 93.91% when pulling 023bc4d on stevendaniels:cleanup_excelx into 67381a6 on roo-rb:master.

stevendaniels added a commit that referenced this pull request May 28, 2015
@stevendaniels stevendaniels merged commit 84982e0 into roo-rb:master May 28, 2015
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.

3 participants