Checking whether this pull request makes sense #136
Conversation
Update branch
Update branch
|
Hi Lucas, |
|
Did I mess something up... |
I you look at the diff, I made a bunch of changes that are hiding in merges... |
src/pyglimer/waveform/download.py
Outdated
| @@ -813,7 +817,7 @@ def downloadwav( | |||
| # # Create handler to the log | |||
| if log_fh is None: | |||
| fh = logging.FileHandler(os.path.join('logs', 'download.log')) | |||
| fh.setLevel(logging.INFO) | |||
| fh.setLevel(logging.info) | |||
There was a problem hiding this comment.
this is not correct. logging.info is a function, whereas logging.INFO is the actual logging level.
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## dev #136 +/- ##
==========================================
- Coverage 33.74% 33.71% -0.03%
==========================================
Files 37 37
Lines 4839 4855 +16
Branches 863 865 +2
==========================================
+ Hits 1633 1637 +4
- Misses 3112 3124 +12
Partials 94 94
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
looks good. Seems like there is an error with lint (So probably some cosmetic thing). We should probably have a separate action for linting and not run it together with unittest, sorry for that. |
|
@lsawade Can you check to fix the things that make the unittests fail? If I remember correctly it was only a listing issue. But otherwise I think this is almost mergeable. |
|
I have made some more mini-updates that I'm going to push and as part of that I'm going to fix the formatting errors. |
…t work if multiple stations are used.
|
again some linting problems, but after you fixed this, we can merge. |
Hi Peter,
I think I didn't pull something from your branch at some point, and now it's fast-forwarding and rewinding and stuff. There were 0 merge conflicts, but before merging this I/we should go through at least some of the changes, just to see whether they make sense.