Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Real-time status about sync details #569

Merged
merged 17 commits into from
Aug 3, 2022
Merged

Real-time status about sync details #569

merged 17 commits into from
Aug 3, 2022

Conversation

boatbomber
Copy link
Member

@boatbomber boatbomber commented Jul 3, 2022 •
edited
Loading

Resolves #418.

image

Features:

  • Human-readable timestamp text (just now, 3 seconds ago, 1 minute ago, 2 minutes ago)
  • How many changes were applied by the latest patch
  • Timestamp text live updates (and slows the update freq as it gets older)
  • Combines patches that are within a second of each other, to more readably handle burst changes

@OverHash
Copy link

OverHash commented Jul 3, 2022

Do users need to know how many changes have occurred in a given time period? I feel like just displaying "last synced just now" or "last synced 5 minutes ago" is enough.

@boatbomber
Copy link
Member Author

Do users need to know how many changes have occurred in a given time period? I feel like just displaying "last synced just now" or "last synced 5 minutes ago" is enough.

@OverHash The #418 issue specifically asked to know a count, and no one disagreed. That said, I do think that a simple last since readout would solve the most common use case put forth in the issue- it informs you that the plugin is still working. However, it's possible that there could be cases where it's only syncing partially, which wouldn't be exposed by just a timestamp.

@boatbomber boatbomber marked this pull request as ready for review July 3, 2022 21:55
Kampfkarren
Kampfkarren reviewed Jul 3, 2022
View reviewed changes
plugin/src/App/init.lua Outdated Show resolved Hide resolved
@LPGhatguy LPGhatguy added the status: needs review This work is mostly done, but just needs work to integrate and merge it. label Jul 9, 2022
kuphg
kuphg approved these changes Jul 23, 2022
View reviewed changes
Copy link

@kuphg kuphg left a comment

Choose a reason for hiding this comment

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

I see nothing that could be improved on in terms of code, I think this should be merged as soon as possible.

LPGhatguy
LPGhatguy approved these changes Aug 3, 2022
View reviewed changes
Copy link
Contributor

@LPGhatguy LPGhatguy left a comment

Choose a reason for hiding this comment

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

Very clean, thank you!

plugin/src/App/StatusPages/Connected.lua
@@ -13,6 +13,26 @@ local BorderedContainer = require(Plugin.App.Components.BorderedContainer)

local e = Roact.createElement

local AGE_UNITS = { {31556909, "year"}, {2629743, "month"}, {604800, "week"}, {86400, "day"}, {3600, "hour"}, {60, "minute"}, }
Copy link
Contributor

Choose a reason for hiding this comment

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

"last synced one year ago" would be a pretty funny message to happen in practice

@LPGhatguy LPGhatguy merged commit e110f37 into rojo-rbx:master Aug 3, 2022
@boatbomber boatbomber deleted the display-sync-details branch August 3, 2022 22:59
Dekkonot pushed a commit to UpliftGames/rojo that referenced this pull request Jan 11, 2024
@boatbomber @LPGhatguy
Real-time status about sync details (rojo-rbx#569)
96a66d0 
* Rough prototype of patch info display

* Remove extra newline

* Switch to binding

* Update slower for older timestamps

* Batch patches within a second of each other

* Fix indentation

* Less wasteful refresh hz

* More apt variable name

Co-authored-by: Lucien Greathouse <me@lpghatguy.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kampfkarren Kampfkarren Kampfkarren left review comments

@kuphg kuphg kuphg approved these changes

@LPGhatguy LPGhatguy LPGhatguy approved these changes

Assignees
No one assigned
Labels
status: needs review This work is mostly done, but just needs work to integrate and merge it.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rojo Plugin should offer real-time status about sync details
5 participants
@boatbomber @OverHash @LPGhatguy @Kampfkarren @kuphg