Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

Conversation

@drwasho
Copy link
Member

@drwasho drwasho commented Oct 11, 2018

The changes in these commits cover the following:

  1. Renames title to status
  2. Adds type to indicate if the post is a 'comment' or 'repost' etc, if so:
  3. Adds reference to point to the original post
  4. Adds channels to allow a social post to be directed at a specific topic

This is a backwards incompatible change for nodes that have used posts; however since posts hasn't rolled out in desktop and has only been tested in a handful of nodes run by the developers, the impact should be negligible.

The changes in these commits cover the following:

1. Renames `title` to `status`
2. Adds `type` to indicate if the post is a 'comment' or 'repost' etc, if so:
3. Adds `reference` to point to the original post
4. Adds `channels` to allow a social post to be directed at a specific topic

This is a backwards incompatible change for nodes that have used posts; however since posts hasn't rolled out in desktop and has only been tested in a handful of nodes run by the developers, the impact should be negligible.
@drwasho drwasho requested review from a user, cpacia and placer14 October 11, 2018 05:51
The max lengths were increased as:

1. Tags could be used to reference a listing
2. Channels could be used to reference a store or listing

This length gives us more than enough space if IPNS addresses increase beyond what they are now.
@drwasho
Copy link
Member Author

drwasho commented Oct 11, 2018

Some additional information on the channels field; the terminology is a callback to IRC channels (not to be confused with our future feature 'curation channels'). A feed of posts can be constructed based on posts that specify certain channel.

For example, I could create a post advertising my listing or store in the nike channel... curators could post their review of a sneaker in the nike channel etc.

Tags add some granularity to finding posts within a channel too. Keeping with the example, I may blast out a social post in nike and sneaker, but my post carries a tag yeezy or drwasho review.

@drwasho drwasho force-pushed the drwasho.posts.refactor branch from 89326a9 to 68a1867 Compare October 11, 2018 08:47
@drwasho drwasho force-pushed the drwasho.posts.refactor branch from 68a1867 to d7fb7b9 Compare October 11, 2018 11:45
@coveralls
Copy link

coveralls commented Oct 11, 2018

Coverage Status

Coverage decreased (-0.02%) to 34.338% when pulling 546e8ac on drwasho:drwasho.posts.refactor into 682c63a on OpenBazaar:master.

@hoffmabc
Copy link
Member

Let’s make sure when this code is merged that we have some adequate documentation accompanying it. I don’t want to lose the description of this stuff.

@drwasho
Copy link
Member Author

drwasho commented Oct 15, 2018

@hoffmabc documentation added!

Copy link
Member

@placer14 placer14 left a comment

Choose a reason for hiding this comment

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

A few changes requested, please. Thanks @drwasho. I also wouldn't object to some unit testing around these some of the critical parts of the code (like slug generation maybe or validation checking to name some examples).

Tested some of these errors which appeared to work normally.
@drwasho drwasho mentioned this pull request Oct 18, 2018
This will then be used in listings and posts when creating content.
- Documentation will be moved to the OBIP for posts
- After merging `master` into this branch, which had a change in the protobuf, we needed updated `pb.go` files for the changes to the protobuf from posts
@drwasho
Copy link
Member Author

drwasho commented Oct 24, 2018

Documentation moved to OBIP: OpenBazaar/obips#37

Copy link
Member

@placer14 placer14 left a comment

Choose a reason for hiding this comment

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

One more round of changes which I'll apply. Thanks @drwasho

},
"title": "test1",
"status": "test1",
"postType": "POST",
Copy link
Member

Choose a reason for hiding this comment

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

This should be a constant as above.

core/posts.go Outdated
}
for _, channel := range post.Channels {
if len(channel) > PostChannelsMaxCharacters {
return ErrPostChannelsLengthLongerThanMax
Copy link
Member

Choose a reason for hiding this comment

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

And this error should be ErrPostChannelsLongerThanMax instead.

core/posts.go Outdated

// Channels
if len(post.Channels) > MaxPostChannels {
return ErrPostChannelsLongerThanMax
Copy link
Member

Choose a reason for hiding this comment

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

The error looks like it should be ErrPostChannelsLengthLongerThanMax.

Copy link
Member

Choose a reason for hiding this comment

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

I see. They're right, just the name allows them to be confused with their similarly named counterpart.

core/posts.go Outdated
// Tags
if len(post.Tags) > MaxPostTags {
return fmt.Errorf("Tags in the post is longer than the max of %d characters", MaxPostTags)
return ErrPostTagsLongerThanMax
Copy link
Member

Choose a reason for hiding this comment

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

The name of the errors don't appear to match their content. I had to doublecheck to disambiguate this with ErrPostTagsLengthLongerThanMax.

@placer14 placer14 force-pushed the drwasho.posts.refactor branch from e9042c4 to 4955a9b Compare October 29, 2018 19:37
* origin/master:
  Log error adding key to peerstore
  Update bitcoin/listeners/transaction_listener.go
  Improved grammar and fixed capitalization in the main README file.
  Fixed import error
  Update Documentation section (PR OpenBazaar#1252)
  Remove unused Python modules in qa/chat.py
  Improve README: fix grammar
  CLEANUP: Fix misspellings.
  Enable goimports linter rules
  HandleOfflineRelay save public key

 Conflicts:
	core/core.go
	core/listings.go
@placer14 placer14 force-pushed the drwasho.posts.refactor branch from 7e60eee to a480939 Compare October 30, 2018 02:27
@placer14 placer14 force-pushed the drwasho.posts.refactor branch from a480939 to 546e8ac Compare October 30, 2018 02:42
@placer14 placer14 merged commit e9f9fc0 into OpenBazaar:master Oct 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants