j6k4m8 / goosepaper

generate and deliver a daily newspaper to you or your remarkable tablet
Apache License 2.0
271 stars 24 forks source link

Multiple fixes, feature adds, cleanup. Also partially completes issue 44… #48

Closed sedennial closed 2 years ago

sedennial commented 3 years ago

… #44

Fixes:

Cleanup/Housekeeping:

New features:

codecov[bot] commented 3 years ago

Codecov Report

Merging #48 (6616f6d) into master (50dc649) will decrease coverage by 4.25%. The diff coverage is 2.52%.

:exclamation: Current head 6616f6d differs from pull request most recent head 1ad5383. Consider uploading reports for the commit 1ad5383 to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
- Coverage   39.09%   34.83%   -4.26%     
==========================================
  Files          19       19              
  Lines         550      620      +70     
==========================================
+ Hits          215      216       +1     
- Misses        335      404      +69     
Impacted Files Coverage Δ
goosepaper/__main__.py 0.00% <0.00%> (ø)
goosepaper/auth.py 0.00% <ø> (ø)
goosepaper/multiparser.py 0.00% <0.00%> (ø)
goosepaper/rss.py 37.50% <ø> (ø)
goosepaper/upload.py 0.00% <0.00%> (ø)
setup.py 0.00% <ø> (ø)
goosepaper/util.py 81.81% <50.00%> (-1.21%) :arrow_down:
goosepaper/goosepaper.py 41.48% <100.00%> (ø)

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 236978e...1ad5383. Read the comment docs.

sedennial commented 3 years ago

I do hesitate a little bit to have upload-only code in here (i.e. a code path that JUST uploads and has nothing to do with paper generation). I worry that we're starting to mix code purposes a little, and maybe upload-specific workflows should use something like rmapy directly.

I kinda thought about that too, but one reason I did it was if you have a lot of goosepapers (I have five separate ones which take a while to run) the user could have them created, and then have a different upload process entirely if they wanted. Which is how I was using it initially. While right now the users probably have the ability to use rmapy to do standalone uploads, it would be nice for less programmatic users to be able to use GP to handle that too. Right now I'm also using pandoc to produce some daily outputs from non-RSS sites (https://isc.sans.org) for example and I'm using GP to handle the upload.

Also in line with the possibility of sending update, documentation, errors, etc. to the RM, this gives GP the ability to compile multiple logs from multiple runs, convert them, and then upload them.

All of that said I'm not going to dig in my heels if you want to take it out. I'll just keep it in my private branch. :)

j6k4m8 commented 2 years ago

@sedennial — I had some stylistic nitpicks which is why it's taken me so long to get back to this — and then I realized that if I really was going to be such a stickler, there were plenty of places to fix my own code first :) Very sorry for the lag here.

My main priority is making sure that this is backwards-compatible with prior code (i.e., someone can update their goosepaper install and keep using their scripts as-is). So far I think this is true, but insight welcome! Otherwise, I think this is ready to merge, right?