Skip to content

Switch StakingRequestProvider to Redux - #407

Merged
eric-zero-dev merged 2 commits into
developfrom
feature/ericlow/switch-staking-provider-redux
Mar 11, 2022
Merged

Switch StakingRequestProvider to Redux#407
eric-zero-dev merged 2 commits into
developfrom
feature/ericlow/switch-staking-provider-redux

Conversation

@eric-zero-dev

Copy link
Copy Markdown
Contributor

Associated Notion Card


1. Pull request checklist

  • Notion card has been moved to the Code Review column
  • Notion card has a link to this PR
  • A reviewer has been assigned to the Notion card

2. PR type

3. What is the old behaviour?

4. What is the new behaviour?

5. Other information

@netlify

netlify Bot commented Mar 9, 2022

Copy link
Copy Markdown

✔️ Deploy Preview for wilderworld-app ready!

🔨 Explore the source changes: 1a00311

🔍 Inspect the deploy log: https://app.netlify.com/sites/wilderworld-app/deploys/622a7c140c30b900090b5ee7

😎 Browse the preview: https://deploy-preview-407--wilderworld-app.netlify.app

@eric-zero-dev
eric-zero-dev requested a review from domw30 March 9, 2022 20:23

@domw30 domw30 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice 👍 Approving with a couple of questions 😁

@@ -0,0 +1,210 @@
import { useMemo, useCallback } from 'react';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Leave this for now as we can do this in it's own PR - We should extract the error text in the file to a global error constant file and replace with all instances in the codebase, I think we re-use them quite a bit.

Comment thread src/lib/hooks/useStaking.tsx Outdated
import { useStakingController } from 'lib/hooks/useStakingController';
import { useStakingRedux } from 'store/staking/hooks';

export type UseStakingReturn = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

just a curious question mate...what is the difference using type over interface for the function props? I ask as I've been instructed in previous projects to use interface

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@domw30 You are right. I sometimes forget it :(

DOMAIN_REQUEST_AND_CONTENTS_1,
DOMAIN_REQUEST_AND_CONTENTS_2,
} from './staking.mockData';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice testing 🔥

fulfilled: DomainRequestAndContents[];
};

/**

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I could be wrong but the comment and type names below look like they have slight typos? is stacking supposed to be staking?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call - This is typo

@eric-zero-dev
eric-zero-dev merged commit 33a426e into develop Mar 11, 2022
@eric-zero-dev
eric-zero-dev deleted the feature/ericlow/switch-staking-provider-redux branch March 11, 2022 22:41
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.

2 participants