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

[Tiny/refactoration] Move HomeDBManager to its own folder #1076

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

fflorent
Copy link
Collaborator

@fflorent fflorent commented Jul 1, 2024

This is a tiny and purely technical PR to move HomeDbManager to homedb/index.ts.

@fflorent fflorent changed the title [Tiny] Move HomeDBManager to own folder [Tiny] Move HomeDBManager to its own folder Jul 1, 2024
@fflorent fflorent force-pushed the move-homedbmanager-to-own-folder branch from 65902ea to 3dac83e Compare July 1, 2024 11:25
@fflorent fflorent changed the title [Tiny] Move HomeDBManager to its own folder [Tiny/refactoration] Move HomeDBManager to its own folder Jul 1, 2024
@berhalak
Copy link
Contributor

berhalak commented Jul 1, 2024

Hi @fflorent. I like the idea of extracting modules from our lib folder. But I slightly prefer leaving the name of this file as it was. It is much easier to find it and it follows common patterns we already have.

@fflorent
Copy link
Collaborator Author

fflorent commented Jul 1, 2024

Hi @berhalak!

Hi @fflorent. I like the idea of extracting modules from our lib folder. But I slightly prefer leaving the name of this file as it was. It is much easier to find it and it follows common patterns we already have.

OK, I prefer having the module inside the folder where there already is another module (homedb/UsersManager.ts).
That being said, I agree that a consistent pattern is more interesting.
So is it fine as it is to you (in such case, I may just close the PR)? Or should I rename the folder to ./homedbmanager?

@berhalak
Copy link
Contributor

berhalak commented Jul 2, 2024

Hi @berhalak!

Hi @fflorent. I like the idea of extracting modules from our lib folder. But I slightly prefer leaving the name of this file as it was. It is much easier to find it and it follows common patterns we already have.

OK, I prefer having the module inside the folder where there already is another module (homedb/UsersManager.ts). That being said, I agree that a consistent pattern is more interesting. So is it fine as it is to you (in such case, I may just close the PR)? Or should I rename the folder to ./homedbmanager?

Hi @fflorent,

I'm fine in moving it to a lib/homedb folder, and grouping all things related to database management there (so HomeDbManager and UserManger). But I would just leave the name of the file as it is, so instead of index.ts I'd still call it HomeDbManager.ts

@fflorent fflorent added the anct label Jul 4, 2024
@fflorent fflorent force-pushed the move-homedbmanager-to-own-folder branch from 3dac83e to 50fbf4d Compare July 4, 2024 15:17
@fflorent
Copy link
Collaborator Author

fflorent commented Jul 4, 2024

Hi @berhalak !

OK, I have made the changes as requested.

@fflorent fflorent requested a review from berhalak July 4, 2024 15:18
@fflorent fflorent force-pushed the move-homedbmanager-to-own-folder branch from 50fbf4d to fa4bd08 Compare July 4, 2024 15:25
@fflorent fflorent force-pushed the move-homedbmanager-to-own-folder branch from fa4bd08 to e7596b8 Compare July 4, 2024 16:07
Copy link
Contributor

@berhalak berhalak left a comment

Choose a reason for hiding this comment

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

Thanks @fflorent !

@berhalak berhalak merged commit 786ba6b into gristlabs:main Jul 5, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants