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

Export comments #1471

Merged
merged 8 commits into from Jan 16, 2019
Merged

Export comments #1471

merged 8 commits into from Jan 16, 2019

Conversation

rhymes
Copy link
Contributor

@rhymes rhymes commented Jan 6, 2019

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Documentation Update

Description

I've added comments to the export and changed/updated the copy.

Related Tickets & Documents

#1461

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Added to documentation?

  • docs.dev.to
  • readme
  • no documentation needed

@rhymes
Copy link
Contributor Author

rhymes commented Jan 6, 2019

The [WIP] is for three reasons:

  1. the list of allowed attributes needs to be finalized. @benhalpern here listed those to remove. If reactions_count shouldn't be included then we should probably remove it from the export of the articles as well. Unless it works differently among the two resources

  2. In the copy I changed data with content which seems less impersonal

  3. I was wondering if in the comments export we should add some information about what the comment is attached to. Not the ancestry/tree information, but the path (or URL?) of the commentable. Otherwise the comment is a little "lost" in the export

@benhalpern
Copy link
Contributor

@rhymes

  1. Yeah, you're right. Not highly sensitive info, but that shouldn't have been included. Our bad on the proofread.
  2. 👌
  3. Yeah that makes sense.

Comments are attached to something but from the export that is not easily identifiable. We add the "commentable_path" to give some context.
@rhymes rhymes changed the title [WIP] Export comments Export comments Jan 15, 2019
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Jan 15, 2019
@rhymes
Copy link
Contributor Author

rhymes commented Jan 15, 2019

@benhalpern

I've removed the field reactions_count from both articles and comments and added commentable_path as an extra field in the comments export with the path (not the URL) for the resource the comment is attached to

Copy link
Contributor

@benhalpern benhalpern left a comment

Choose a reason for hiding this comment

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

Looks great

@pr-triage pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Jan 16, 2019
@benhalpern benhalpern merged commit a0c9576 into forem:master Jan 16, 2019
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: reviewed-approved bot applied label for PR's where reviewer approves changes labels Jan 16, 2019
@rhymes
Copy link
Contributor Author

rhymes commented Jan 16, 2019

Thanks!

@rhymes rhymes deleted the rhymes/export-comments branch January 16, 2019 18:47
@maestromac maestromac mentioned this pull request Jan 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: merged bot applied label for PR's that are merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants