Godot: HEADUPS: Information about the big code reformat and what it means for your PRs

Created on 5 Mar 2017  路  10Comments  路  Source: godotengine/godot

Just a heads-up. As mentioned previously we're going to apply the new clang-format code formatting (#7547) to the whole code base, which means that virtually every line will get some whitespace changes, effectively breaking all pending PRs.

Don't freak out though, it's possible to fix PRs (or local code you may have) with some clever "manual" rebasing, e.g. as described here: https://engineering.mongodb.com/post/succeeding-with-clangformat-part-3-persisting-the-change/

I will post some instructions on how to do it when I've tested it locally. My plan is to start apply the reformat to my local master branch, and then try this manual rebase and document it for the lambda branch.

If you have unmerged work in your local master branch, I would advise that you branch it off to a backup branch, so that when I push the reformatted code you can just reset your master branch to the remote without losing work.

discussion

All 10 comments

BTW, whether the reformat should be also done in the 2.1 branch is open to discussion. It would likely help cherry-picking changes between 2.1 and master, but at the same time such a big cosmetic change in a soon-to-be-EOL'ed branch might not be worth the trouble. WDYT?

When is "soon"? We're talking upwards of three months at least? If so, why not apply it to 2.1 anyway.

I think that as long as 2.1 is still maintained, it should be as easy as possible to backport relevant stuff.

An scenario to consider before abandoning 2.1 is a games develope with it and just or about to be released. Its life could span a few years and during that time it may need bug fixes from Godot or updates to the platform support code.

The only option of letting 2.1 go will be when that lifespan has expired or when 3.0/3.1 supports GL ES 2.0 and provides a reliable and painless conversion of projects from 2.1.

"Reliable and painless" would still require some degree of manual work (probably substantial, especially for larger projects). GLES 2.0 support in the 3.x release chain might be a more realistic argument for when to drop 2.1.x support.

Yes, too ambitious. But what about games already developed with 2.1,
starting its maintenance now?

Done in 5dbf180. I'll handle the 2.1 branch later.

To all contributors: please make sure that you respect the clang-format style when you make new PRs. For this, you can:

Note that I've used clang-format 3.9.1, so it's best if we all stick to the 3.9.x branch for now, in case 4.x brings new rules and thus inconsistency in the style.

Since I have no idea for a better place yet I'll post it here. I'll describe the workflow I used to "rebase" the lambda branch on top of the current reformatted master branch.

Obviously a git rebase, git merge or git pull --rebase would yield impossible to solve conflicts, so I used another approach, inspired from https://engineering.mongodb.com/post/succeeding-with-clangformat-part-3-persisting-the-change/. But since I don't understand half of that git speak there, I'll just put it in simple word here on a real life example.

This assumes that the feature branch has only 1 commit, which makes it somewhat easier. It should be doable iteratively for multiple commits, but it's more pain - so unless it's very important to keep the commit history of your branch, you might consider squashing them all together first.

  1. Checkout the branch you want to update.
  2. Rebase it on the last non-reformatted commit, to be sure that you would have solved potential conflicts from before the reformat. That commit is 45438e9918d421b244bfd7776a30e67dc7f2d3e3, so git rebase 45438e9918d421b244bfd7776a30e67dc7f2d3e3 should do the trick.
  3. Apply clang-format to the modified files of the rebased commit. You can find them with git show --name-only, and apply clang-format with clang-format -style=file -i <path/to/files>.
  4. Be brave: copy the modified files outside the repo, e.g. in ~/tmp.
  5. Checkout the reformat commit of the master branch, i.e. 5dbf1809c6e3e905b94b8764e99491e608122261, so git checkout 5dbf1809c6e3e905b94b8764e99491e608122261
  6. Make a new branch, e.g. git checkout -b lambda-reformat.
  7. Copy the files you had put in a temporary folder on top of this new branch. You should now have the "rebased" content of your feature branch, clean on top of the reformatted branch.
  8. Commit the changes, ideally by faking the author if that's not you and the date if relevant using GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE.
  9. If needed, rebase that new branch on top of the master branch as usual.
  10. Push that to a new branch on your remote, or force push to your previous feature branch if you're sure of your changes or want to update your PR.

So here's the (tidied up) excerpt from my bash history for a real life example:

git checkout lambda
git rebase 45438e9918d421b244bfd7776a30e67dc7f2d3e3
git show --name-only
clang-format -style=file -i modules/gdscript/*.{cpp,h}
mkdir ~/tmp/gdscript && cp modules/gdscript/*.{cpp,h} ~/tmp/gdscript
git checkout -- .
git checkout 5dbf1809c6e3e905b94b8764e99491e608122261
git checkout -b lambda-format
mv ~/tmp/gdscript/* modules/gdscript/ -f
git status
git diff
git add modules/gdscript/
GIT_AUTHOR_NAME="dbsGen" GIT_AUTHOR_EMAIL="[email protected]" GIT_AUTHOR_DATE="Fri, 30 Oct 2015 16:51:30 +0800" git commit -m "Add lambda expressions and function objects in GDScript

Squashed version of #2704.

Edit by R茅mi Verschelde: Squashed all changes in one commit and fixed some
indentation issues. Also applied clang-format to match the new master branch."
git log
git push origin lambda-format:lambda -f

Reopening to keep some visibility about the stuff in there.

This should be explained in CONTRIBUTING.md, no?

Was this page helpful?
0 / 5 - 0 ratings