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

refactor: split files #56

Merged
merged 8 commits into from
Oct 10, 2023
Merged

refactor: split files #56

merged 8 commits into from
Oct 10, 2023

Conversation

jly36963
Copy link
Contributor

@jly36963 jly36963 commented Oct 10, 2023

Description

  • Split functions into separate files
  • Group files by kind
  • Imports include file extensions

@p9f
Copy link
Collaborator

p9f commented Oct 10, 2023

I would really like to try to keep the tests close to the source (locality of behaviour mindset), do you think we could consider ways to achieve it?

One idea would be to group the utilities by types and then in each folder have all the source + tests:

native-utilities/
  join.ts
  join.test.ts
  ...
word-utilities/
  camelCase.ts
  camelCase.test.ts
  ...
object-utilities/
  camelKeys.ts
  camelKeys.test.ts
  deepCamelCase.ts
  ...

That might help to make the new naming convention clearer.
we could also merge the .type.test.ts and .test.ts files - as they will be quite small because of the split (see this)

@gustavoguichard
Copy link
Owner

I agree with the comments from @p9f .
Also.. I kind of hate camel cased filenames. I'd rather keep them in kebab case bc it works best in file system world - I've has troubles with that more than once. Do you have a strong reason to use them camel cased?

@jly36963 jly36963 marked this pull request as ready for review October 10, 2023 13:22
@jly36963
Copy link
Contributor Author

jly36963 commented Oct 10, 2023

I couldn't think of.a better name for the additional folder

tsconfig.json Outdated Show resolved Hide resolved
@gustavoguichard
Copy link
Owner

gustavoguichard commented Oct 10, 2023

I couldn't think of.a better name for the additional folder

I think the words method belongs to that folder even though it is used in the casing folder... maybe utils (which is also quite broad) sounds better?

Co-authored-by: Guga Guichard <gustavoguichard@gmail.com>
@p9f
Copy link
Collaborator

p9f commented Oct 10, 2023

maybe we remove the casing folder and:

  • move toLowerCase and toUpperCase to the native folder
  • having words-utils or just words for all the utils functions based on the words method? so that it's clear what the difference is between toLowerCase (not word based) and the other case methods (word based)?

@gustavoguichard
Copy link
Owner

gustavoguichard commented Oct 10, 2023

move toLowerCase and toUpperCase to the native folder
Agreed

For the words method, I believe it should belong to the same folder as truncate and reverse from #71 which are not casing functions nor native ones but popular string utilities from other libs.

@jly36963
Copy link
Contributor Author

For sure, will make those changes during lunch today. If you can think of any other feedback, let me know.
I'm hoping to get this merged quickly so as to avoid a lot of merge conflicts

@jly36963 jly36963 marked this pull request as draft October 10, 2023 14:48
@gustavoguichard
Copy link
Owner

gustavoguichard commented Oct 10, 2023

@jly36963 to avoid conflicts I'll stop my activity until this PR is merged =)

@jly36963 jly36963 marked this pull request as ready for review October 10, 2023 17:54
Copy link
Owner

@gustavoguichard gustavoguichard left a comment

Choose a reason for hiding this comment

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

LGTM

@gustavoguichard gustavoguichard merged commit bf9345d into main Oct 10, 2023
4 checks passed
@gustavoguichard gustavoguichard deleted the split-files branch October 10, 2023 18:07
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.

None yet

3 participants