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

feat(core-api): more params for delegates search endpoint #2184

Merged
merged 11 commits into from Mar 6, 2019

Conversation

Projects
None yet
3 participants
@dated
Copy link
Contributor

dated commented Feb 28, 2019

Proposed changes

Adds support for more options to the delegates search endpoint.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works

dated added some commits Feb 27, 2019

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Feb 28, 2019

@dated Thanks for submitting this pull request, a maintainer will get back to you shortly!

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Feb 28, 2019

@air1one @faustbrian @supaiku0 - please review this in the next few days. Be sure to explicitly select labels so I know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Feb 28, 2019

The ci/circleci: test-node11-1 job is failing as of 215c6574490e733d02897831b9fcbf6c3a0762da. Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@dated dated force-pushed the dated:delegates-search branch from cb213f3 to a35a6e2 Feb 28, 2019

@faustbrian
Copy link
Collaborator

faustbrian left a comment

Looks good but API tests should be added for searches with the new parameters.

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Feb 28, 2019

@dated Your pull request needs some changes. Please wait for a comment from one of our developers for more information.

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Feb 28, 2019

@dated Your pull request doesn't have a test case, which is a requirement for it to be merged. Please provide it and one of the developers will review it before merging.

@dated

This comment has been minimized.

Copy link
Contributor Author

dated commented Feb 28, 2019

objectHasSomeOwnProperty could be moved to the utils as that kind of check is performed elsewhere also.

I'll add the respective API tests tonight.

@faustbrian faustbrian changed the title feat: more params for delegates search endpoint feat(core-api): more params for delegates search endpoint Mar 3, 2019

@faustbrian
Copy link
Collaborator

faustbrian left a comment

I would move objectHasSomeOwnProperty as hasSomeProperty into core-utils.

@faustbrian faustbrian changed the base branch from 2.3 to 2.2 Mar 4, 2019

@faustbrian faustbrian changed the base branch from 2.2 to develop Mar 4, 2019

dated and others added some commits Mar 4, 2019

@dated

This comment has been minimized.

Copy link
Contributor Author

dated commented Mar 4, 2019

  ● Guard › __determineOffence › should return a 1 year suspension for "Blacklisted"

    expect(received).toBe(expected) // Object.is equality

    Expected: 525600
    Received: 527040

is this fixed on develop?

@faustbrian

This comment has been minimized.

Copy link
Collaborator

faustbrian commented Mar 4, 2019

It's an odd issue caused by the upcoming leap year it seems, you can copy 884c894 to develop.

faustbrian and others added some commits Mar 4, 2019

@dated dated marked this pull request as ready for review Mar 5, 2019

@dated dated requested a review from kristjank as a code owner Mar 5, 2019

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Mar 6, 2019

A collaborator has approved this PR. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait.

Thank you for your contribution!

@faustbrian

This comment has been minimized.

Copy link
Collaborator

faustbrian commented Mar 6, 2019

@dated SQLite build failed on CircleCI. Could you also update the docs with the new parameters?

@dated

This comment has been minimized.

Copy link
Contributor Author

dated commented Mar 6, 2019

All good now! PR for the docs has been submitted.

@faustbrian faustbrian merged commit 3377337 into ArkEcosystem:develop Mar 6, 2019

6 checks passed

ci/circleci: test-node10-0 Your tests passed on CircleCI!
Details
ci/circleci: test-node10-1 Your tests passed on CircleCI!
Details
ci/circleci: test-node10-2 Your tests passed on CircleCI!
Details
ci/circleci: test-node11-0 Your tests passed on CircleCI!
Details
ci/circleci: test-node11-1 Your tests passed on CircleCI!
Details
ci/circleci: test-node11-2 Your tests passed on CircleCI!
Details
@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Mar 6, 2019

Your pull request has been merged and marked as tier 2. It will earn you $50 USD.

@dated dated deleted the dated:delegates-search branch Mar 6, 2019

vasild added a commit that referenced this pull request Mar 6, 2019

Merge remote-tracking branch 'ArkEcosystem/core/develop' into blockid
* ArkEcosystem/core/develop:
  feat(core-api): more params for delegates search endpoint (#2184)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.