Skip to content

feat: add options object to specify custom params for feed.items()#1264

Open
Androz2091 wants to merge 7 commits intodilame:masterfrom
Androz2091:params-limit
Open

feat: add options object to specify custom params for feed.items()#1264
Androz2091 wants to merge 7 commits intodilame:masterfrom
Androz2091:params-limit

Conversation

@Androz2091
Copy link
Copy Markdown

It is now possible to retrieve as many messages as desired per thread when fetching direct inbox items or direct thread items

@Androz2091
Copy link
Copy Markdown
Author

Androz2091 commented Aug 27, 2020

Could close #1263

Copy link
Copy Markdown
Collaborator

@Nerixyz Nerixyz left a comment

Choose a reason for hiding this comment

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

You should use the class as the input for the options not the request method.

Comment thread src/feeds/direct-inbox.feed.ts Outdated
Comment thread src/feeds/direct-pending.feed.ts Outdated
Comment thread src/feeds/direct-thread.feed.ts Outdated
@Androz2091
Copy link
Copy Markdown
Author

I tried to change that, should we add @Expose()? Should we use camelCase instead of snake_case for things like thread_message_limit?

Comment thread src/core/feed.factory.ts Outdated
Comment thread src/feeds/direct-inbox.feed.ts Outdated
Comment thread src/feeds/direct-inbox.feed.ts Outdated
Comment thread src/feeds/direct-pending.feed.ts Outdated
Comment thread src/core/feed.factory.ts Outdated
@Androz2091 Androz2091 requested a review from Nerixyz August 31, 2020 05:58
@bayoumymac
Copy link
Copy Markdown
Contributor

@Nerixyz I am new in here, I have had 2 prs open since july and I was wondering if there's an issue I can help with as they haven't gotten any comments. #1224 #1231

@Nerixyz
Copy link
Copy Markdown
Collaborator

Nerixyz commented Sep 1, 2020

@Nerixyz I am new in here, I have had 2 prs open since july and I was wondering if there's an issue I can help with as they haven't gotten any comments. #1224 #1231

I've seen the PRs already. They seem fine to me. I haven't tested them though.

@Expose()
private seqId: number;

public limit?: number;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Specify the default values here instead of using ?? operator below

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The same for another feeds

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.

4 participants