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

Add code formatting and translations check #4362

Merged
merged 3 commits into from
Mar 15, 2021
Merged

Add code formatting and translations check #4362

merged 3 commits into from
Mar 15, 2021

Conversation

microdev1
Copy link
Collaborator

Ran tools/codeformat.py script ported from MicroPython with slight modifications:

  • Modified included directories.
  • Added a check for empty list.
  • Revert // | back to //| for docs.
diff --git micropython/tools/codeformat.py circuitpython/tools/codeformat.py
--- micropython/tools/codeformat.py
+++ circuitpython/tools/codeformat.py
@@ -35,21 +35,20 @@ import subprocess
 # Relative to top-level repo dir.
 PATHS = [
     # C
+    "devices/**/*.[ch]",
+    "drivers/bus/*.[ch]",
     "extmod/*.[ch]",
-    "extmod/btstack/*.[ch]",
-    "extmod/nimble/*.[ch]",
-    "lib/mbedtls_errors/tester.c",
     "lib/netutils/*.[ch]",
     "lib/timeutils/*.[ch]",
     "lib/utils/*.[ch]",
-    "mpy-cross/*.[ch]",
-    "ports/*/*.[ch]",
-    "ports/windows/msvc/**/*.[ch]",
-    "py/*.[ch]",
+    "mpy-cross/**/*.[ch]",
+    "ports/**/*.[ch]",
+    "py/**/*.[ch]",
+    "shared-bindings/**/*.[ch]",
+    "shared-module/**/*.[ch]",
+    "supervisor/**/*.[ch]",
     # Python
-    "drivers/**/*.py",
-    "examples/**/*.py",
-    "extmod/**/*.py",
+    "extmod/*.py",
     "ports/**/*.py",
     "py/**/*.py",
     "tools/**/*.py",
@@ -116,13 +115,14 @@ def fixup_c(filename):
                         # This #-line does not need dedenting.
                         dedent_stack.append(-1)
                 else:
-                    if dedent_stack[-1] >= 0:
-                        # This associated #-line needs dedenting to match the #if.
-                        indent_diff = indent - dedent_stack[-1]
-                        assert indent_diff >= 0
-                        l = l[indent_diff:]
-                    if directive == "endif":
-                        dedent_stack.pop()
+                    if dedent_stack:
+                        if dedent_stack[-1] >= 0:
+                            # This associated #-line needs dedenting to match the #if.
+                            indent_diff = indent - dedent_stack[-1]
+                            assert indent_diff >= 0
+                            l = l[indent_diff:]
+                        if directive == "endif":
+                            dedent_stack.pop()
 
             # Write out line.
             f.write(l)
@@ -161,7 +161,7 @@ def main():
             file_args = list(itertools.islice(files, N))
             if not file_args:
                 break
-            subprocess.check_call(cmd + file_args)
+            subprocess.call(cmd + file_args)
 
     # Format C files with uncrustify.
     if format_c:
@@ -171,6 +171,11 @@ def main():
         batch(command, lang_files(C_EXTS))
         for file in lang_files(C_EXTS):
             fixup_c(file)
+        # Revert "// |" back to "//|"
+        subprocess.call(
+            "find shared-bindings ports/*/bindings -name '*.c' -exec sed -i 's/\/ |/\/|/' {} \;",
+            shell=True,
+        )
 
     # Format Python files with black.
     if format_py:

@tannewt
Copy link
Member

tannewt commented Mar 11, 2021

Thanks for doing this! Looks like it needs to be rerun again. Please ping be when it's ready and we'll get it in. We'll also need to teach folks how to run it on any new or pending code as well.

Is there a way to check the formatting in the CI via pre-commit?

@microdev1
Copy link
Collaborator Author

Please ping when it's ready and we'll get it in.

Ya... this needs some coordination to avoid conflicts.

Is there a way to check the formatting in the CI via pre-commit?

Yes, we can have that.

This currently fails CI at Test all with the following error which I am unable to debug:

Run MICROPY_CPYTHON3=python3.8 MICROPY_MICROPYTHON=../ports/unix/micropython_coverage ./run-tests -j1
Traceback (most recent call last):
  File "./run-tests", line 615, in <module>
    main()
  File "./run-tests", line 606, in main
    res = run_tests(pyb, tests, args, base_path, args.jobs)
  File "./run-tests", line 286, in run_tests
    upy_float_precision = int(run_feature_check(pyb, args, base_path, 'float.py'))
ValueError: invalid literal for int() with base 10: b'64_0x0a_\n'
Error: Process completed with exit code 1.

@jepler
Copy link
Member

jepler commented Mar 11, 2021

@microdev1 the formatting changes in py/qstrdefs.h are wrong and break qstr generation. This leads to the print-ending newline character being printed as the literal sequence _0xa0_ instead.

@jepler
Copy link
Member

jepler commented Mar 15, 2021

@microdev1 ping -- did the above comment enable you to move forward or are you still stuck with the same problem when reverting py/qstrdefs.h?

- add formatting check
- add translations check
@microdev1
Copy link
Collaborator Author

Is there a way to check the formatting in the CI via pre-commit?

@tannewt I have added formatting check to pre-commit along with translations check.

did the above comment enable you to move forward...

@jepler Thanks! for digging into this. :)
The qstrdefs issue is resolved. I disabled the formatting on it.
// *FORMAT-ON/OFF* for C and # fmt: off/on for Python can be used to limit formatting.

@microdev1
Copy link
Collaborator Author

Seems like CI is still failing at Test all but the tests are different this time... any ideas?

@jepler
Copy link
Member

jepler commented Mar 15, 2021

The problem is due to the added line in cmd_showbc.py. As far as I can tell, when the expected file has something like

########
  bc=\\d\+ line=155

it searches for a line that matches this regular expression, and then checks all the subsequent lines until the next ######## for also matching as regular expressions. However, line 155 has become line 166 due to the added line, and so forth

https://gist.github.com/8293402cffa2d8dfc3bbc80786988151 fixes this test by updating all the line numbers.

@microdev1
Copy link
Collaborator Author

Thanks! @jepler. That solved the issue.

@microdev1 microdev1 changed the title Code formatting Add code formatting and translations check Mar 15, 2021
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

I hadn't noticed the translations check part -- thank you for everything!

@jepler jepler merged commit 05ed179 into adafruit:main Mar 15, 2021
@microdev1 microdev1 deleted the code-formatting branch March 15, 2021 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants