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

Cannot merge old feature because of code reformattings #3464

Closed
psychocoderHPC opened this issue Dec 8, 2020 · 6 comments
Closed

Cannot merge old feature because of code reformattings #3464

psychocoderHPC opened this issue Dec 8, 2020 · 6 comments
Labels
documentation regarding documentation or wiki discussions

Comments

@psychocoderHPC
Copy link
Member

psychocoderHPC commented Dec 8, 2020

If you developed a feature before we switched to clang-format and you have issues with rebasing and merging the following easy 😃 steps will solve your pain.

# Note: code below assumes clang-format-12 from llvm 12.0.1
git checkout dev
git fetch mainline   # assuming you have PIConGPU repo stored as remote named `mainline`
git rebase mainline/dev
cp .clang-format .. # save format file

git checkout <feature branch>  # <> is only used to show the name length
cp ../.clang-format .
find include/ share/picongpu/ share/pmacc -iname "*.def" \
  -o -iname "*.h" -o -iname "*.cpp" -o -iname "*.cu" \
  -o -iname "*.hpp" -o -iname "*.tpp" -o -iname "*.kernel" \
  -o -iname "*.loader" -o -iname "*.param" -o -iname "*.unitless" \
  | xargs clang-format-12 -i
git commit -a -m "clang-format"
rm .clang-format

git checkout <hash where feature branch split of>  # e.g. 00eb7516968fbc982e020b8e6895d772357b644b
git checkout -b topic-base
cp ../.clang-format .
find include/ share/picongpu/ share/pmacc -iname "*.def" \
  -o -iname "*.h" -o -iname "*.cpp" -o -iname "*.cu" \
  -o -iname "*.hpp" -o -iname "*.tpp" -o -iname "*.kernel" \
  -o -iname "*.loader" -o -iname "*.param" -o -iname "*.unitless" \
  | xargs clang-format-12 -i
git commit -a -m "clang-format"
rm .clang-format

git checkout <feature branch>
git diff topic-base..HEAD > ../feature.diff

git checkout dev
git checkout -b topic-newFeatureWithFormating
git apply ../feature.diff

# you have now all changes unstaged and format available
git status
# open a new PR or overwrite your old origin branch (please validate the new branch first)

@ComputationalRadiationPhysics/picongpu-developers

@psychocoderHPC psychocoderHPC added the documentation regarding documentation or wiki discussions label Dec 8, 2020
@PrometheusPi
Copy link
Member

@psychocoderHPC Thank you for posting this detailed description.

@sbastrakov
Copy link
Member

sbastrakov commented Dec 8, 2020

Note: for the simple case of adding new stuff and (almost) not modifying existing code lines, one can get away with just git reset -> git add -> git clang-format with maybe a couple of manual fixes. However one has to use their judgement of what is and isn't a simple case.

@n01r n01r changed the title Can not merge old feature because of code reformatings Cannot merge old feature because of code reformattings Dec 16, 2020
@psychocoderHPC psychocoderHPC pinned this issue Jan 21, 2021
finnolec added a commit to finnolec/picongpu that referenced this issue Oct 28, 2021
Use issue ComputationalRadiationPhysics#3464 to format the backgroundfield laser from @BeyondEspresso
@BrianMarre
Copy link
Member

BrianMarre commented Jun 16, 2022

Please note, the exact formatting rules of clang-format are dependent on the version of clang-format.
Therefore check your clang-format version using 'clang-format --version' before formatting the code.

Current used version is clang-format-12 from llvm 12.0.1, the bug fix version does actually make a difference as I found out recently.

@psychocoderHPC could you update your example to the currently used version?

@sbastrakov
Copy link
Member

@BrianMarre thanks, I edited the first message to reflect the current state, also made a fix in the docs.

@BrianMarre
Copy link
Member

@psychocoderHPC since we switched to precommit could we close this issue?

@PrometheusPi
Copy link
Member

I agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation regarding documentation or wiki discussions
Projects
None yet
Development

No branches or pull requests

4 participants