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.

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

handle quote tweet-style mentions #582

Merged
merged 3 commits into from
Dec 24, 2015
Merged

Conversation

kylewm
Copy link
Contributor

@kylewm kylewm commented Dec 23, 2015

original post discovery needs to run recursively on attachments (if
the attachment belongs to our user) to find the original URL for the
attachment

ref #504

original post discovery needs to run recursively on attachments (if
the attachment belongs to our user) to find the original URL for the
attachment

ref snarfed#504
@kylewm
Copy link
Contributor Author

kylewm commented Dec 23, 2015

@kylewm
Copy link
Contributor Author

kylewm commented Dec 23, 2015

uhm, but it also backfed a response to tantek for his own tweet :/

https://bridgy-kwm.appspot.com/log?start_time=1450912031&key=agxzfmJyaWRneS1rd21yNQsSCFJlc3BvbnNlIid0YWc6dHdpdHRlci5jb20sMjAxMzo2Nzc2ODY5NTQ3NzE4MDAwNjQM

EDIT: ok for better or for worse this is the existing behavior, not a regression. here is the same tweet backfed from bridgy proper a week ago:

https://brid.gy/log?start_time=1450409434&key=aglzfmJyaWQtZ3lyNQsSCFJlc3BvbnNlIid0YWc6dHdpdHRlci5jb20sMjAxMzo2Nzc2ODY5NTQ3NzE4MDAwNjQM

@snarfed
Copy link
Owner

snarfed commented Dec 23, 2015

yup, whee. afaik that's one of the edge case casualties of the post-#51 OPD new world order. for many of them (like that one), i think we didn't even agree amongst ourselves what the right behavior was.

so to confirm, that's entirely unrelated to this PR?

@kylewm
Copy link
Contributor Author

kylewm commented Dec 23, 2015

Yep, unrelated! I was worried it might be a regression at first but it wasn't

On December 23, 2015 3:33:33 PM PST, Ryan Barrett notifications@github.com wrote:

yup, whee. afaik that's one of the edge case casualties of the post-#51
OPD new world order. for many of them (like that one), i think we
didn't even agree amongst ourselves what the right behavior was.

so to confirm, that's entirely unrelated to this PR?


Reply to this email directly or view it on GitHub:
#582 (comment)

'original post discovery on attachment found originals=%s, mentions=%s',
att_origs, att_mentions)
mentions.update(att_origs)
mentions.update(att_mentions)
Copy link
Owner

Choose a reason for hiding this comment

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

should we do this? i'm not sure. even with salmentions, if A mentions B, and B mentions C, that doesn't mean A transitively mentions C, does it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right, I'll remove that line

@snarfed
Copy link
Owner

snarfed commented Dec 24, 2015

you were right! this definitely seems straightforward. thanks for doing it!

@snarfed
Copy link
Owner

snarfed commented Dec 24, 2015

oh also, corresponds to snarfed/granary#69, just for the record.

@snarfed
Copy link
Owner

snarfed commented Dec 24, 2015

oh, and updating the "what gets sent back" section in the docs! always a fun last step, at least for me.

@kylewm kylewm merged commit 5d942ee into snarfed:master Dec 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants