Skip to content

Conversation

@iammac360
Copy link

Request for Comments!

My initial implementation of telnet server. I implemented this because when I'm using watch to poll the updates, it doesn't render the ascii charts properly on my terminal. Not sure you encountered this guys. It is still buggy at the moment(crash prone) and it still doesn't have any proper form validation. Also, the form UI is still plain and clunky. Maybe anyone here with experience with BlessedJS and telnet programming can make this better because I'm still in the process of tinkering and learning this technologies 😅.

asciicast

@vercel
Copy link

vercel bot commented Apr 2, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/warengonzaga/covid19-tracker-cli/79ww6j9gx
✅ Preview: https://covid19-tracker-cli-git-fork-iammac360-feature-telnet.warengonzaga.now.sh

Copy link
Member

@warengonzaga warengonzaga left a comment

Choose a reason for hiding this comment

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

How to use your implementation?

@iammac360
Copy link
Author

Just run node telnet.js and on another terminal window type telnet localhost 2300.

@warengonzaga warengonzaga added this to the Version 4 milestone Apr 3, 2020
@warengonzaga warengonzaga added the tweak Issue/Pull Request for Tweaks label Apr 3, 2020
@warengonzaga warengonzaga requested review from ianvizarra and warengonzaga and removed request for warengonzaga April 3, 2020 02:56
@ianvizarra
Copy link
Contributor

This is nice :)

Copy link
Contributor

@ianvizarra ianvizarra left a comment

Choose a reason for hiding this comment

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

updated attribute is available in Country API now

telnet.js Outdated
// empty, country, history, chartType
const [_x, country, history, chartType] = query.split('/'),
countryData = await axios.get(`${apiBaseURL}/countries/${country}`),
all = await axios.get(`${apiBaseURL}/all`),
Copy link
Contributor

Choose a reason for hiding this comment

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

We can omit this call now since we only call this to get the "updated" attribute.
The country API has updated attribute available now.

so instead of u.updated use d.updated on line 37 same with line 45

Copy link
Author

Choose a reason for hiding this comment

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

Hi I already removed the extra API call on country tracker. Can you please verify if this is correct. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

@ianvizarra please verify this and close if necessary so we can move forward. Will improve this later on.

Copy link
Member

Choose a reason for hiding this comment

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

outdated nato...

@warengonzaga
Copy link
Member

@ianvizarra can we close this? It seems outdated the code base... submit PR again...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tweak Issue/Pull Request for Tweaks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants