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

Apply clang-format #886

Closed
wants to merge 1 commit into from
Closed

Apply clang-format #886

wants to merge 1 commit into from

Conversation

kxl-adsk
Copy link

@kxl-adsk kxl-adsk commented Nov 4, 2020

Time to start leveraging clang-format in maya-usd. Below are updated instructions from #843

Instructions for applying clang-format to an outstanding branch / PR

These instructions assume that:

  • $MAYA_USD_REPO points to the root directory of a checkout of the maya-usd source repository
  • $COMMIT_BEFORE_FORMAT points at the commit in the dev branch immediately before clang-format is applied (this is 5cfeb1b)
  • $FORMAT_COMMIT points at the commit in the dev branch where clang-format was applied (this is 6bd514d)
  • $FORMAT_BRANCH points at the head of the branch, containing any commits after $FORMAT_COMMIT (dev branch)
  • $YOUR_BRANCH is the name of the branch which does not have clang-format applied, but want to merge dev (with clang-format) into

Download and install clang-tools 10.0 (generally included as part of llvm):

Merge + apply clang-format to your branch (these instructions assume Linux, you may need to adapt to other OSes):

cd $MAYA_USD_REPO
git checkout $YOUR_BRANCH
git merge $COMMIT_BEFORE_FORMAT
# resolve any merge errors that may have resulted from previous command
# this will technically create a merge in git, but won't actually change the
# state of any files
git merge -s ours $FORMAT_COMMIT
# now, apply the formatting to your branch...for Linux:
find . -regextype posix-egrep -regex "$(echo '.*('$(echo $(cat .clang-format-include) | tr ' ' '\|')')')" -exec clang-format -i {} \+
# for MacOS
# find -E . -regex "$(echo '.*('$(echo $(cat .clang-format-include) | tr ' ' '\|')')')" -exec clang-format -i {} \+
# ...then ammend the commit, to make this the new "merged" result
git commit -a --amend --no-edit
# now merge in any of the latest stuff, that came AFTER the clang-format changes
git merge $FORMAT_BRANCH
# OPTIONAL: do a sanity check, to make sure that the diff of your branch to
#           the format branch now only contains "substantive" changes (ie, not a
#           bunch of formatting changes!)
git diff $FORMAT_BRANCH
# ...or, if you have a gui difftool set up in your .gitconfig:
git difftool $FORMAT_BRANCH

@kxl-adsk kxl-adsk added the build Related to building maya-usd repository label Nov 4, 2020
}
#endif

#endif // UFE_V2_FEATURES_AVAILABLE

} // namespace ufe
} // namespace MayaUsd
} // namespace MAYAUSD_NS_DEF
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a shame. The macro name is really not useful here.

Copy link
Author

Choose a reason for hiding this comment

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

Clang-format auto-generates this comment and currently, we are giving it a macro. Something that may be newer versions of clang-format will handle better.

Comment on lines +20 to +21
#include <QtCore/QSortFilterProxyModel>
#include <QtWidgets/QTreeView>
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like these should be part of group 6 and thus be moved to line 25?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch. This is because we don't recognize mayaUsdUI in https://github.com/Autodesk/maya-usd/blob/dev/.clang-format#L56. I will fix this and create a new PR.

Copy link
Author

Choose a reason for hiding this comment

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

PR opened for it #888

@kxl-adsk kxl-adsk closed this Nov 4, 2020
@kxl-adsk kxl-adsk deleted the kxl-adsk/apply_clang_format branch November 4, 2020 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to building maya-usd repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants