PR II: Pixar Mesh write translator/utilities refactoring

[HamedSabri-adsk]

PART II

• Rename meshUtil(*.h,*.cpp) to meshReadUtils(*.h,*.cpp).
• Follow coding guideline for function name ( lowerCamelCase ).

[mattyjams]

As an intermediate commit, this looks good to me.

meshUtil.h has a mix of read and write utilities, so I was going to gripe that moving all of it to meshReadUtils.h might not really make sense. I peeked ahead and saw that it looks like that's addressed in later PRs though.

In the future, it would probably be better to collapse all of these separate PRs into a single PR with multiple commits so that it's easy to see the incremental progression commit by commit, but the final collapsed result is still easy to see. This also ensures that all of the discussion is kept in one place. This becomes particularly important in cases like this where the same code and files are touched in multiple commits.

[HamedSabri-adsk]

In the future, it would probably be better to collapse all of these separate PRs into a single PR with multiple commits so that it's easy to see the incremental progression commit by commit, but the final collapsed result is still easy to see. This also ensures that all of the discussion is kept in one place. This becomes particularly important in cases like this where the same code and files are touched in multiple commits.

The main reason for making multiple PRs was because sometimes there are so many comments and conversation in one PR that makes it super easy for one to get lost in there. My main goal was to make the PRs short and compilable and tried to communicate it in PR description that certain changes will be address in other PRs.

I am not a big fan of Github review system and in my workflow always review PRs locally in my preferred Git GUI when the changes are more than 3-4 files.

Sadly I don't think this would change anytime soon and doesn't seem we can use better code review systems ( e.g code collaborator ) in this public project.

[mattyjams]

Ideally, the wrapMeshUtil() function below would be renamed to wrapMeshWriteUtils() as well to match the file name, which would also require a change in the module.cpp.

[HamedSabri-adsk]

@mattyjams see 45f0f51

[mattyjams]

The main reason for making multiple PRs was because sometimes there are so many comments and conversation in one PR that makes it super easy for one to get lost in there. My main goal was to make the PRs short and compilable and tried to communicate it in PR description that certain changes will be address in other PRs.

I am not a big fan of Github review system and in my workflow always review PRs locally in my preferred Git GUI when the changes are more than 3-4 files.

Sadly I don't think this would change anytime soon and doesn't seem we can use better code review systems ( e.g code collaborator ) in this public project.

If there are going to be many comments and conversations, I would greatly prefer that they be collected in a single PR rather than spread across many PRs. And when the volume of change grows this large, there's likely to be conversation. The best way to control that is to keep PRs as small and focused as possible, and to organize the commits in each PR so that the steps needed to achieve the end result are clear.

When the same file is changed across a series of PRs, reviewers end up wasting time reviewing code in an earlier PR that may be completely different or totally irrelevant in a later PR.

GitHub's review system was clearly not intended to handle hundreds of files, so again, the best course is to keep PRs small. Reviewing your own changes before posting them is good practice, but effectively requiring your reviewers to do that also because a PR is so large puts additional burden on them. It also slows the turnaround time down for you the more work reviewers need to do.