GheisMohammadi
commented
3 months ago Thanks for the PR. It is a great work. There are a few points here to mention:
-
The PR is quite extensive, with over thousands lines of changes, but the description is only three words long. I see a lot more changes rather than updating go version. Could you please provide a more detailed description? This will help us understand the purpose and scope of the changes better.
-
It looks like you’re bringing in over 5000 lines of code from the
harmony-one/harmony-testrepository and placing them in a new folder calledtests. We already have atestfolder, and having bothtestandtestscould be confusing. Additionally, duplicating code across repositories might make it very hard to maintain consistency. Could you explain the reasoning behind this approach? Perhaps we could consider using Docker to clone theharmony-testrepository and use it directly, which might simplify maintenance. -
The commit titles are quite brief and a bit unclear (e.g., "test", "tests", "1.21.11"). Could you please provide more descriptive commit messages? This would greatly help in understanding the changes at a glance and maintaining the project history.
-
There’s a commit titled "Fixed trailing zeroes" which seems to actually update dependencies instead. Clear and accurate commit messages are important to avoid confusion, so it would be helpful to correct this.
-
I see that the tests for this PR are still ongoing. It might be best to wait until all tests are complete and verified to ensure there are no unexpected issues. We have to have this PR on test-net for awhile to ensure consistency and stability.
Frozen
Golang version 1.22