-
Notifications
You must be signed in to change notification settings - Fork 430
[CI] Fix CPP Format Issue #3344
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
Conversation
67ca544 to
5261d05
Compare
The format test for CPP was done through a bash script which calls make format under the hood. The issue was that if make format fails, it thinks that the formatting is good, which is bad. I have fixed the script so that it actually fails if make format fails. The thing that was causing make format to fail was that the dependencies that VPR needs to build were not installed. Added these dependencies.
5261d05 to
e16f263
Compare
The formatter was failing on some python files since they were using the Python2 syntax for the print statements instead of the Python3 syntax.
Run make format-py to fix all Python format issues.
Now that the CI test is fixed, I have resolved all outstanding format issues.
AmirhosseinPoolad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this Alex! LGTM.
P.S. We have so many Python 2 scripts! Is there a tool that can check a .py file and check if it's compatible with python 2 or 3?
|
|
||
|
|
||
| def usage(): | ||
| print USAGE_TEXT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this PR:
Woah! is that python 2? Do we even run this anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many of the scripts in VTR are not run by the CI. I do not have any hard data on this, but I think most Python scripts outside of vtr_task's python libs are not run at all.
Another issue is that Python is an interpreted language; so a Python2 syntax file can be executed by a Python3 interpreter and not crash unless it hits a magic if statement which prints something. I actually saw a file with this exact problem.
@AmirhosseinPoolad I agree. I do not like it either, I am not aware of any tool; however, I think the Python linter was running the python code in some form since it drew my attention to this Python2 syntax issues (and had me fix them) without actually executing the code. However, this process has to be limited since Python is interpreted and the types of variables are not known until run time. |





The format test for CPP was done through a bash script which calls make
format under the hood. The issue was that if make format fails, it
thinks that the formatting is good, which is bad. I have fixed the
script so that it actually fails if make format fails.
The thing that was causing make format to fail was that the dependencies
that VPR needs to build were not installed. Added these dependencies.