-
Notifications
You must be signed in to change notification settings - Fork 25
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.
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
%activity agent #3219
%activity agent #3219
Conversation
We also implement logic for finding the "floor", timestamp of oldest unread activity, for a certain category (everything, or a specific thread). (The reader should also note that the struggle with "time id" types continues...)
5e9c2a1
to
7f17b6f
Compare
desk/sur/activity.hoon
Outdated
[%club p=id:club] | ||
[%club p=@uvH] | ||
== | ||
+$ timid time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just time-id
?
desk/app/activity.hoon
Outdated
^- (unit (unit cage)) | ||
?+ pole [~ ~] | ||
[%x ~] | ||
``activity-full+!>([stream indices]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would maybe add unreads summary to this
desk/desk.bill
Outdated
@@ -7,5 +7,6 @@ | |||
%groups-ui | |||
%channels | |||
%channels-server | |||
%activity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should probably leave it out and turn it on manually until we're ready for it to run in production. This won't be released for a while but it would be nice to let it flow through the release pipeline, but not be on by default.
desk/sur/activity.hoon
Outdated
++ eon ((on time event) lte) | ||
++ emp ((mp time event) lte) | ||
++ mep ((on time event-parent) lte) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might need clearer names here
overall this looks good, I think we can address some of the comments here and get a test thread then move on to integrating it, because that's where will find real issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, good to see tests! let's move on to integrating
PR Checklist