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

Added utility folder to common-lib #220

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sidtohan
Copy link
Contributor

Description

I think it would be wise decision to include the utils folder in the common lib module since there are many utilities which are shared among modules. For example,
both the attendance and calendar module use the formatDate function.
There may be many more such repeated functions in the code so I think having them all in common lib increases re usability.

Changes

  1. Added utils folder in the common lib module
    The directory structure is now as follows:
    image

Note:

Currently the typings for utils is not being produced by common lib. I think it may be a config issue so any help there may be appreciated.

@sidtohan sidtohan force-pushed the sidtohan/utils/commonlib branch from e0ce05f to 02ff2b3 Compare July 16, 2022 11:53
@coolbung
Copy link
Collaborator

Please put a comment in each of these hooks / functions to explain the purpose of each.

Copy link
Member

@Shruti3004 Shruti3004 left a comment

Choose a reason for hiding this comment

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

Changes LGTM @sidtohan, needs testing.

sidtohan added 2 commits July 24, 2022 00:11
* Many utilities are shared across modules, having them all in common lib may help
* For now only the utils folder has been added there, config must be modified to produce appropriate typings
* Added comments to all functions, custom hooks and types to briefly describe their use
* Removed some redundant utilities that were not needed when the code is in utils folder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants