Pegase745 / sqlalchemy-datatables

SQLAlchemy integration of jQuery DataTables >= 1.10.x (Pyramid and Flask examples)
MIT License
159 stars 67 forks source link

Revamp a bit #110

Closed Pegase745 closed 5 years ago

codecov[bot] commented 5 years ago

Codecov Report

Merging #110 into master will increase coverage by 0.19%. The diff coverage is 93.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
+ Coverage   93.68%   93.87%   +0.19%     
==========================================
  Files           1        5       +4     
  Lines         190      196       +6     
==========================================
+ Hits          178      184       +6     
  Misses         12       12
Impacted Files Coverage Δ
datatables/column_dt.py 100% <100%> (ø)
datatables/__init__.py 100% <100%> (+6.31%) :arrow_up:
datatables/search_methods.py 89.36% <89.36%> (ø)
datatables/clean_regex.py 92.85% <92.85%> (ø)
datatables/datatables.py 94.95% <94.95%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d1e686a...9f6757d. Read the comment docs.

tdamsma commented 5 years ago

My take at change log from what I just saw

And per my suggestion

louking commented 5 years ago

Are there not users still "stuck" on 2.7 because of some limited support from external packages? I know I've been afraid to try to migrate.

On Tue, Nov 20, 2018 at 5:01 AM Thijs Damsma notifications@github.com wrote:

My take at change log from what I just saw

  • Convert tests to pytest
  • Reorganize code
  • Modernize Flask example
  • Modernize pyramid example
  • Change and apply different linting style
  • Rename .jinja2 templates to .html
  • Update to python 3.6

And per my suggestion

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Pegase745/sqlalchemy-datatables/pull/110#issuecomment-440214475, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1agqdjt1YT_mQT5YPKFCy5tKN8tlffks5uw9LbgaJpZM4YPXQs .

Pegase745 commented 5 years ago

@tdamsma thanks for your inputs. I felt sorry ignoring the project's need for maintenance for so long :) My main concern was to fix our old issue with the Travis tests that failed for no reasons at that time when using an SQLite database. I may need help in integrating the current important PRs in the revamp branch, in order to address some of the issues. I keep the support for python2 because I see it everyday how much companies are still afraid, or don't have time to migrate due to large base code. For the time being, it doesn't cost too much to keep the compatibility imho

tdamsma commented 5 years ago

Are you sure about that last commit, to remove asserts? They are at the core of pytest. Nothing wrong with using asserts for testing, and pytest is built around providing nice error messages for assert statements. Wouldn't be surprised if that is broken now. See https://github.com/fossasia/query-server/issues/332

tdamsma commented 5 years ago

About python 2, I guess if is no bother you might as well leave it in, but I haven't touched any python 2 code in over 6 years.

Pegase745 commented 5 years ago

@tdamsma good to know that ! thanks, I'll revert that, and even stop using codacy due to the 7 hours for each review..

tdamsma commented 5 years ago

@Pegase745, You're welcome, keep up the good work!

tdamsma commented 5 years ago

@Pegase745, do you think this branch can be released as is?

Pegase745 commented 5 years ago

will do this tonight then :)