← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/automate_clang-format into lp:widelands

 

Review: Approve

pretty hard to review this diff with the formatting changes. I found two nits, I think otherwise this is ready to merge. 

Diff comments:

> === renamed file 'utils/fix_lua_tabs.py' => 'utils/fix_formatting.py'
> --- utils/fix_lua_tabs.py	2015-10-31 12:11:44 +0000
> +++ utils/fix_formatting.py	2016-12-03 18:02:09 +0000
> @@ -2,29 +2,35 @@
>  # -*- coding: utf-8 -*-
>  
>  
> -"""The code base had inconsistent usage of tabs/spaces for indenting in Lua
> +"""The code base had inconsistent usage of tabs/spaces for indenting in Lua.
> +
>  files. Spaces were more prominent - and I prefer them over tabs. So I wrote
>  this small script to fix leading tabs in Lua files to spaces.
>  
>  It also saves files in unix file endings ("\r\n") and strips empty lines at the
>  end of files and whitespace characters at the end of lines.
> +
> +After fixing the Lua tabs, this script also executes clang-format over the src directory and pyformat over the utils directory.

overlong line.

> +
>  """
>  
>  import argparse
>  import os
>  import re
>  import sys
> +from subprocess import call
>  
>  LEADING_TABS = re.compile(r'^\s*\t+\s*')
>  PYTHON3 = sys.version_info >= (3, 0)
>  SPACES_PER_TAB = 3
>  
> +
>  def parse_args():
> -    p = argparse.ArgumentParser(description=
> -        "Fix common whitespace errors in Lua files. Recurses over all Lua files."
> -    )
> +    p = argparse.ArgumentParser(
> +        description='Fix common whitespace errors in Lua files, run clang-format over the code base and pyformat over the utils directory. Recurses over all relevant files.')

overlong line. You can split strings in python line in c: "hello world" is the same as "hello"
" world"

>      return p.parse_args()
>  
> +
>  def read_text_file(filename):
>      """Reads the contens of a text file."""
>      if PYTHON3:


-- 
https://code.launchpad.net/~widelands-dev/widelands/automate_clang-format/+merge/312287
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/automate_clang-format.


References