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

Rename datatables #22

Merged
merged 9 commits into from
Aug 20, 2018
Merged

Conversation

Prakriti-nith
Copy link
Contributor

Rename module DataTables to View. Now DataTable will be created as:

Daru::View::DataTable.new(data, options)

@coveralls
Copy link

coveralls commented Aug 1, 2018

Pull Request Test Coverage Report for Build 27

  • 49 of 51 (96.08%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-2.7%) to 97.345%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/daru/data_tables/display/iruby_notebook.rb 4 6 66.67%
Totals Coverage Status
Change from base Build 22: -2.7%
Covered Lines: 440
Relevant Lines: 452

💛 - Coveralls

@coveralls
Copy link

Pull Request Test Coverage Report for Build 18

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Totals Coverage Status
Change from base Build 17: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 18

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Totals Coverage Status
Change from base Build 17: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

@Prakriti-nith
Copy link
Contributor Author

@Shekharrajak can you please review this PR?

@Shekharrajak
Copy link
Owner

We need to use coveralls properly, to see the correct coverage report. After that PR will be merged.

@Prakriti-nith
Copy link
Contributor Author

@Shekharrajak ping!

@Shekharrajak Shekharrajak merged commit 3649783 into Shekharrajak:master Aug 20, 2018
@Shekharrajak
Copy link
Owner

Refer all the changes because of this PR.

@Prakriti-nith
Copy link
Contributor Author

Changes are made to the rails examples in this PR only. Corresponding changes in daru-view are made in this PR.

@Shekharrajak
Copy link
Owner

It seems we didn't change the folder structure. :o . It must be inside daru/view or daru/view/datatables . What do you think ? @Prakriti-nith

@Prakriti-nith
Copy link
Contributor Author

@Shekharrajak I tried changing the folder structure earlier but changing the filename from data_tables.rb to view.rb or keeping it in view folder generates the following error:

uninitialized constant Daru::View::DataTable

in rails controller.
So, I reverted back the folder structure to the previous one. I tried it again now too but facing the same problem.

@Shekharrajak
Copy link
Owner

@Prakriti-nith , did you try in console or IRuby notebook ? I seems you did't try daru/view/datatables means daru -> view -> data_tables (means current daru -> data_tables content will go to daru -> view -> data_tables folder) and daru -> view -> data_tables.rb will be our current (daru -> data_tables.rb). So you just have to modify the daru -> data_tables.rb require path accordingly.

@Prakriti-nith
Copy link
Contributor Author

@Shekharrajak I tried daru/view/datatables but when I add the gem in Gemfile of the rails like this:

gemspec  :path => '../../'

in dummy_rails it throws the error

uninitialized constant Daru::View::DataTable

However, it is works when I require daru/view/data_tables in application_controller.

@Shekharrajak
Copy link
Owner

@Prakriti-nith , did you try in console or IRuby notebook ?

Did you try ?

However, it is works when I require daru/view/data_tables in application_controller.

I guess you didn't follow this

you just have to modify the daru -> data_tables.rb require path accordingly.

@Shekharrajak
Copy link
Owner

Any update? Your conclusion will help me.

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