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

Feature/use storage hook #45

Merged

Conversation

Christiantyemele
Copy link
Collaborator

No description provided.

@Christiantyemele
Copy link
Collaborator Author

@stephane-segning requesting for review on pull request

@stephane-segning
Copy link
Contributor

Which ticket is it related to please @Christiantyemele ?

Copy link
Contributor

@stephane-segning stephane-segning left a comment

Choose a reason for hiding this comment

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

This won't work. We want a storage system capable of being used everywhere in our app. For that, we need to leverage the capabilities of react, by using react context or redux. You should document that before writing code. Plus, your test file is not explicit enough (it's just an example – you could do that in the doc). Remove that too.

docs/useStorage.md Outdated Show resolved Hide resolved
power-pay-frontend/src/Hooks/useStorage.ts Outdated Show resolved Hide resolved
power-pay-frontend/src/Hooks/useStorage.ts Outdated Show resolved Hide resolved
power-pay-frontend/src/Tests/useStorageTest.tsx Outdated Show resolved Hide resolved
@Christiantyemele
Copy link
Collaborator Author

Hello sir @stephane-segning i have updated the useStorage docs and requesting for review before proceeding.

Copy link
Contributor

@stephane-segning stephane-segning left a comment

Choose a reason for hiding this comment

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

Please work on these points

power-pay-frontend/src/Hooks/useStorage.ts Outdated Show resolved Hide resolved
power-pay-frontend/src/Tests/useStorageTest.tsx Outdated Show resolved Hide resolved
docs/useStorage.md Show resolved Hide resolved
@Christiantyemele
Copy link
Collaborator Author

@stephane-segning have done requested changes and requesting review

Copy link
Contributor

@stephane-segning stephane-segning left a comment

Choose a reason for hiding this comment

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

Please focus this only on the documentation of how the useStorage should work, nothing more. Avoid any unuseful comments about react context or redux. If you plan using react context, then explain how to use it to implement the hook.

docs/useStorage.md Outdated Show resolved Hide resolved
docs/useStorage.md Outdated Show resolved Hide resolved
@Christiantyemele
Copy link
Collaborator Author

@stephane-segning i have removed the unnecessary information in the usestorage docs

@Christiantyemele
Copy link
Collaborator Author

@stephane-segning have added visual flow to docs

docs/useStorage.md Outdated Show resolved Hide resolved
@Christiantyemele
Copy link
Collaborator Author

@AssahBismarkabah please can you check useStorage docs if clear enough

@stephane-segning stephane-segning merged commit 3541749 into ADORSYS-GIS:main Mar 8, 2024
5 checks passed
Christiantyemele added a commit to Christiantyemele/forked-e2e-banking-app that referenced this pull request Apr 9, 2024
* docs(useStorage): useStorage hook documentation

* feat(hooks): implementing the useStorage hook

* test(usestorage): simple implementation to test the useStorage hook

* docs(useStorage): useStorage hook documentation

* feat(useStorage): implementing the useStorage hook

* test(useStorage): simple usecase to test useStorage hook

* docs(useStorage): documentation on the choice and reason for the library to use for the useStorage hook

* docs(useStorage): useStorage documentation and library used

* fix(frontend): removing Hooks and Tests folders from frontend

* docs(useStorage): removing unnecessary information in useStorage docs

* docs(useStoragedocs): adding visual representation of flow

* docs(useStorage): visual representation of usestorage docs
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