Prettier auto formatting #233
Conversation
|
This PR aims to enable Prettier code formatting on every commit using a pre-commit hook |
There was a problem hiding this comment.
Thank you, so I have a couple of questions guys: @itegika, @vishwaszadte, @tsimian, @rcamach7
Can you please check my comments?
| const { existsSync, promises: fsPromises } = require("fs"); | ||
| const { join, resolve } = require("path"); | ||
|
|
||
| const excludedDirs = ["node_modules"]; |
There was a problem hiding this comment.
Do you guys think it's a good idea to make this code read and exclude whatever is in the .gitignore file, instead of hard coding node_modules, in case we have something else in the future that we don't want to format (since we don't need to format anything that is not going to be pushed)
There was a problem hiding this comment.
That is a good point, it should exclude anything in the .gitignore file.
There was a problem hiding this comment.
Yes guys, you are right. I was considering the same approach, as it offers more flexibility.
However, considering that:
- We are formatting
.jsfiles only - Among the directories/files listed in
.gitignore, only thenode_modulesdirectory contains.jsfiles. - After checking a list of most frequently "gitignored" files/directories, it seems unlikely that we will encounter other
.jsfiles within.gitignore. - The
.gitignorefile is typically not very dynamic once the project architecture is established. - The execution time using the hardcoded approach is significantly faster.
Taking all these factors into account, I hdecided to go for the hardcoded approach, as it strikes a suitable balance between speed and efficiency. However, if you still prefer to pursue the .gitignore approach, I would be glad to refactor the code accordingly
|
|
||
| console.log("Prettier formatting applied to all JS files."); | ||
|
|
||
| const gitAddCmd = `git add ${allJsFiles.join(" ")}`; |
There was a problem hiding this comment.
Is this going to add (stage) everything even if I try to add one file?
There was a problem hiding this comment.
@lugenx you can add just one file. However, during git commit execution, all .js files will be formatted with prettier, to maintain consistent styling even if local formatting tools/settings are used by some devs.
Then formatted files are staged, ensuring accurate tracking by Git version control, which might not always catch formatting changes as effectively as content changes.
If your added file is the only one with style or content differences, only that one gets pushed
There was a problem hiding this comment.
@lugenx seeing that this PR itself formatted all files to fit the prettier configuration settings, any updated forks making commits after this merge will be formatted the same. So any files that are updated in future PRs should be the only files pushed.
|
hey @itegika, thanks for the PR on automating prettier formatting. here's where I'm pausing though: our main hiccup was ensuring devs didn't have to manually adjust settings to avoid unnecessary diffs. but with the current solution, we're still asking them to manually set a custom path using git config core.hooksPath .git-hooks. so, we still end up with a manual step, which was our initial challenge. got any ideas on streamlining this part? I'm trying to brainstorm some solutions on my end as well. cheers for diving into this! |
|
Thanks for your comment @lugenx I explored various options while working on this PR. The criteria for a good solution included simplicity, predictable output, and flexibility. While options like a postinstall script or a custom post-clone hook were considered, they come with limitations in some cases. Depending on the package manager devs may use or the way they clone the repo, manual setting may still be required. To unify the setup for all, we would need to restrict the tools and workflows devs can use when working on the project. Another option is Dockerizing the app (should only be pursued if it aligns with the project's needs) but formatting automation should't be the main reason for that. The current approach simplifies the setup from multiple steps to just one: setting up the path once, after cloning the repo, reducing the chances of errors in the process. Therefore, I believe that providing clear instructions in our Readme and having devs set up the hooks path is a simple, predictable solution that remains compatible with various workflows and tools. If any of you, @lugenx @tsimian @vishwaszadte @rcamach7, have alternative solutions in mind, I'd appreciate hearing them |
Hi there!
This PR aims to enable Prettier code formatting on every commit using a pre-commit hook.
I'll be glad to answer any further questions. Thanks!