tammoippen / plotille

Plot in the terminal using braille dots.
MIT License
398 stars 17 forks source link

initial text overlay commit with text labels and markers #30

Closed zackbakerusc closed 4 years ago

zackbakerusc commented 4 years ago

Hi Tammo, here's my code. Please let me know if you see anywhere it can be improved.

lgtm-com[bot] commented 4 years ago

This pull request introduces 9 alerts when merging 73f2412a36b1a2e71058b80ae893ac9de35eb2ae into 41f50df2f5b499425465f506a9aae5acf1a39c0b - view on LGTM.com

new alerts:

zackbakerusc commented 4 years ago

I had no idea that circleci cared so much about whitespace. I got your config.yaml working in my fork so I can reproduce the errors and patch them quietly.

On Mon, Mar 9, 2020 at 7:03 PM lgtm-com[bot] notifications@github.com wrote:

This pull request introduces 9 alerts when merging 73f2412 https://github.com/tammoippen/plotille/commit/73f2412a36b1a2e71058b80ae893ac9de35eb2ae into 41f50df https://github.com/tammoippen/plotille/commit/41f50df2f5b499425465f506a9aae5acf1a39c0b

new alerts:

  • 7 for Unused import
  • 2 for Module is imported with 'import' and 'import from'

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tammoippen/plotille/pull/30?email_source=notifications&email_token=AOYEKFZS3Q3NECTHSNIS3WLRGWGW5A5CNFSM4LEUOB42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOJTJ6Q#issuecomment-596849914, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOYEKF2D3ZDVWK66PJX4XA3RGWGW5ANCNFSM4LEUOB4Q .

tammoippen commented 4 years ago

Hi @zackbakerusc ,

First of, thank you for the pull request. Second, sorry for the late reply - for various reasons, i do not get to the pc so often.

For all my projects i apply some style and static analysis of the code, to ensure a certain level of quality. This is tedious at first, but helps in the long run. The whitespace issues are just one part. You do not need circle ci to run on your system to check this:

$ poetry install
$ poetry shell

I will do a proper review later, when i have access to the pc again.

Best, tammo

zackbakerusc commented 4 years ago

I'll close this pull in favor of the branch that passes all checks and tests --