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

Pathlib #43

Merged
merged 14 commits into from
Dec 21, 2019
Merged

Pathlib #43

merged 14 commits into from
Dec 21, 2019

Conversation

ewjoachim
Copy link
Contributor

#29 but there's still work to do

Successful PR Checklist:

  • Tests < though pretty sure not 100% coverage
  • Documentation (optionally: run spell checking)
  • Had a good time contributing? (feel free to give some feedback)

@ewjoachim ewjoachim requested a review from mgu as a code owner December 13, 2019 22:23
@ewjoachim ewjoachim force-pushed the pathlib branch 3 times, most recently from 6bee3fc to f217955 Compare December 13, 2019 22:30
@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

Merging #43 into master will decrease coverage by 32.39%.
The diff coverage is 79.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
- Coverage   88.96%   56.56%   -32.4%     
==========================================
  Files          13       13              
  Lines         580      594      +14     
  Branches       72       81       +9     
==========================================
- Hits          516      336     -180     
- Misses         49      251     +202     
+ Partials       15        7       -8
Flag Coverage Δ
#integration 11.27% <25.97%> (-69.93%) ⬇️
#unit 55.38% <70.12%> (-1.17%) ⬇️
Impacted Files Coverage Δ
septentrion/cli.py 0% <0%> (-79.23%) ⬇️
septentrion/migrate.py 0% <0%> (-79.23%) ⬇️
septentrion/runner.py 79.59% <71.42%> (-15.31%) ⬇️
septentrion/core.py 71.83% <75%> (-3.54%) ⬇️
septentrion/files.py 86.79% <88.09%> (-13.21%) ⬇️
septentrion/configuration.py 76.56% <94.11%> (-12.12%) ⬇️
septentrion/db.py 42.18% <0%> (-54.69%) ⬇️
septentrion/__main__.py 0% <0%> (-33.34%) ⬇️
... and 4 more

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 dc0eb12...b19ada6. Read the comment docs.

@ewjoachim
Copy link
Contributor Author

The drop in coverage is because we stopped counting our acceptance test in the coverage.

Copy link
Contributor

@mgu mgu left a comment

Choose a reason for hiding this comment

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

LGTM.
You left some TODOs in the code, do you plan to fix them in an upcoming PR ?

@ewjoachim
Copy link
Contributor Author

ewjoachim commented Dec 16, 2019

You left some TODOs in the code, do you plan to fix them in an upcoming PR ?

Yes, I'm experimenting a mind blowing new way of coding where I don't try to jam everything in the same PR, looks amazing right ? 😁 I'm also opening tickets, but sometimes, it's so much easier to plant a few TODOs. I plan that we tackle these in the next weeks/months, as part of the current stream of refactor works.

Joachim Jablon and others added 14 commits December 21, 2019 23:19
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
Co-Authored-By: Samuel Hamard <samuel.hamard@people-doc.com>
It's not exactly broken, but VSCode understands it better this way, and
this is equivalent
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