RaSan147
commented
1 year ago Well, commit purpose: try to xxx was a issue because i couldn't build it without github action (mostly coded on mobile and building on my pc failed idk why, mostly dependency and pixi-clone fail maybe), so all those trials and fixes (and also the reason not be able to squash them my skill issue)
commit message: well as said due to mobile upload, commit messages turned out that way. Sorry. If possible you could just copy paste the changes. I've added some comments based on what's going on.
unrelated changes: well yeah the last commit (expression with sound) might look unrelated but they are binded with sound. Like you can mix up expression with motions. but in case you want to do only while playing, you gotta bind it to onFinish event which might not be possible from external access (maybe).
personal info: i don't recall adding such things. if found please notify
thanks, human with no life energy
Hi Ratul Hasan,
I'd like to begin by appreciating your time and effort in developing and contributing this new feature to our project. It is innovative, and the concept you've introduced is very much in line with our strategic objectives.
After reviewing your pull request, there are a few areas that could use some attention to ensure the project's quality and consistency.
Commit Messages: A key part of maintaining a clean and understandable Git history is to ensure commit messages are descriptive and related to the changes made. Currently, some of the commit messages in this pull request are not very descriptive, making it challenging to follow the progress and purpose of the changes. I would suggest revising these commit messages to reflect the changes in each commit more accurately.
Commit Purpose: There are quite a few "try to xxx" commits. While it's completely fine to experiment, I would recommend squashing these experimental commits together or ensuring each commit reflects a meaningful, standalone change in the codebase. This practice would help make the commit history clearer and easier to navigate.
Unrelated Changes: There are also several changes in the pull request that appear unrelated to the primary purpose of adding the new feature. While these changes might be beneficial, it would be best to separate them into distinct pull requests to keep each pull request focused on a single concern. This separation would help us to understand, review, and test each change more effectively.
Changing Default Options: I've noticed some default options in the code were altered without clear explanations or justifications. We usually want to be cautious when changing default options as they can potentially affect existing users. I would appreciate if you could either provide a reasoning behind these changes or revert them.
Personal Information: Lastly, the code currently contains some of your personal information. As a best practice, to protect privacy and maintain professional boundaries, it's important that the codebase doesn't include any contributor's personal data. I would recommend removing this information from the code.
It's great to see your active participation and dedication to the project. Remember that we're all here to learn and improve, and your efforts are truly appreciated. I look forward to seeing the revised version of this pull request and am eager to see how this feature evolves.
Best Regards,
ChatGPTGuan (sorry due to lack of time)