-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Add threaded XML generation with nested conversation structure #74
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
Conversation
|
Related PR: This PR works together with bitcoinsearch/scraper#100 and bitcoinsearch/tldr#526. All PRs should be merged for full threading functionality. |
060a8da to
1cbdaee
Compare
f91d1cd to
6d6bffe
Compare
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.
Your log messages are so nice :)
While I can't really comment on the correctness of the code, I looked over it and it seems fine to me. I'm sorry I cannot review it for bugs.
I took a look at the preview link you provided and it looks great. I didn't think we'd be able to get this feature done so quickly so I appreciate the fast turnaround on something that was very non-trivial. Per the product call last Wednesday I think we still need to:
- change the "Original Post" pill to be a light orange
- expand the background color on mobile for threads with a ton of nested replies
Both those are front end things that I don't should be blockers to merging this. Approving!
|
Ooooo I just noticed something. In the preview link you provided, I expanded all the replies and clicked the second to last one from Saint Wenhao. When I click "Link to Raw post" it takes me to the raw link for what I believe to be the whole thread, not Saint Wenaho's raw reply. Not sure if that issue is related to this PR though. |
I think we had this issue before, you can see it from this thread in prod: It always takes you to the whole thread regardless of which reply you click. Thanks for noticing that! I’ll open an issue for it so we can address it in a separate PR. |
Thanks, fixed here: bitcoinsearch/tldr#526 (comment)
Opened a separate issue for it: bitcoinsearch/tldr#534 |
This PR implements threaded XML generation that preserves reply relationships (parent-child) in generated XML files
thread_depth,thread_position,reply_to_author,parent_idPS: There are many debug logs in the code to help trace job actions in CI. These will be removed once all XML files are updated