Closed NoobTW closed 2 years ago
This pull request introduces 2 alerts when merging d3fdac61bdd95c7ff42e7db373cd3973d42ca8ce into 54ec7677db20c94bbf2f3c65401ea0b9b51ea6e7 - view on LGTM.com
new alerts:
Hello, @NoobTW!
Thanks a lot for your work. That is a great start to fully implement the last seen time feature. Initially, I thought that this feature would require much more effort.
There are some things to mention. In your PR you're deleting the yarn.lock file. Is this was a mistake or you're having some troubles in installing the dependencies? Will you help with the frontend side or should I merge it in the current state in the dev branch?
In your PR you're deleting the yarn.lock file.
Yes, I believe this is a mistake from me because I use npm instead of yarn. Feel free to add it back, or maybe i can commit it later.
Will you help with the frontend side or should I merge it in the current state in the dev branch?
Before merging this PR, i want to check if ping members every 5 minutes is OK? if it's ok then I think you can merge it for now.
As for the frontend side, I would like to help with the frontend side but i don't know how to do it for now. (e.g., Can we get the lastSeen property from the current backend API? I haven't check for that yet.)
Last but not least, thank you for your great work for building this self-host controller and the great UI.
Before merging this PR, i want to check if ping members every 5 minutes is OK? if it's ok then I think you can merge it for now.
I think that this value should be configurable. Anyway, 1 minute is a better default value for troubleshooting. Also, there should be a way to disable the 'last seen time' option.
Pull Request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: #40
What is the new behavior?
pingAll
function inutils/ping.js
to ping every members in a network, and update in database.node-cron
package and ping all networks every 5 minutes.Does this introduce a breaking change?
Other information
There's no changes at the frontend side yet.