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

Adding tests with KWStyle #100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Adding tests with KWStyle #100

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 20, 2016

I made the file for KWStyle fit to the existing code. However, it could be better to be stricter and change the code in consequence.

<VariablePerLine>3</VariablePerLine> <!-- max -->
<StatementPerLine>1</StatementPerLine> <!-- max -->
<Functions>
<regex>[a-z_]</regex>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this ensure that function names are_like_this and notLikeThis?

Copy link
Author

Choose a reason for hiding this comment

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

I hoped so, but it is not the case with at least version 1.0.0+cvs20120330-3.

<regex>[a-z_]</regex>
<length>150</length>
</Functions> <!-- length = number of lines -->
<Comments>
Copy link
Contributor

Choose a reason for hiding this comment

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

This prevents comments that start with // ?

Copy link
Author

Choose a reason for hiding this comment

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

No, I do not know why, and I do not know if there is a feature for that.

@@ -45,6 +46,7 @@ matrix:
env: USE_CMAKE=YES

script:
- if [ ${TRAVIS_OS_NAME} = 'linux' ]; then KWStyle -R -v -gcc -d src; fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not also test the tests directory?

Copy link
Contributor

Choose a reason for hiding this comment

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

I could not figure out from the documentation what -gcc does. What does it do?

Copy link
Author

Choose a reason for hiding this comment

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

I already configured the KWStyle.xml in a lax way (for me). The tests folder would need a laxer configuration to pass currently. Do you prefer a laxer configuration, see that later, or fix the "problems" reported for tests with the current configuration ? I am in favor of seeing that later to have minimal merges (instead of a big one), but you are the boss.

[ -gcc ]
   = Generate errors as GCC format

Copy link
Contributor

Choose a reason for hiding this comment

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

What are the problems currently in the tests directory? Can you run the tests directory with the current configuration so that I may see the issues in Travis-CI? From there it may be better to determine if they should be fixed or are OK as is.

Copy link
Author

Choose a reason for hiding this comment

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

$ KWStyle -R -v -gcc -d tests/
/home/rydroid/workspace/check/tests/Makefile.am:31: error: Line length exceed 186 (max=130)
/home/rydroid/workspace/check/tests/check_check_log.c:316: error: Number of empty lines at the end of files: 2
/home/rydroid/workspace/check/tests/check_check_log_internal.c:76: error: Number of empty lines at the end of files: 2
/home/rydroid/workspace/check/tests/check_check_master.c:124: error: Line length exceed 139 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:125: error: Line length exceed 148 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:126: error: Line length exceed 139 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:127: error: Line length exceed 149 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:128: error: Line length exceed 137 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:129: error: Line length exceed 149 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:130: error: Line length exceed 137 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:131: error: Line length exceed 149 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:159: error: Line length exceed 140 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:160: error: Line length exceed 149 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:161: error: Line length exceed 140 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:162: error: Line length exceed 150 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:163: error: Line length exceed 138 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:164: error: Line length exceed 150 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:165: error: Line length exceed 138 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:166: error: Line length exceed 150 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:194: error: Line length exceed 141 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:195: error: Line length exceed 150 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:196: error: Line length exceed 141 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:197: error: Line length exceed 151 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:198: error: Line length exceed 139 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:199: error: Line length exceed 151 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:200: error: Line length exceed 139 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:201: error: Line length exceed 151 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:204: error: Line length exceed 131 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:213: error: Line length exceed 140 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:214: error: Line length exceed 135 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:227: error: Line length exceed 135 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:235: error: Line length exceed 185 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:242: error: Line length exceed 371 (max=130)
/home/rydroid/workspace/check/tests/check_check_master.c:243: error: Line length exceed 372 (max=130)
/home/rydroid/workspace/check/tests/check_check_sub.c:2237: error: Line length exceed 277 (max=130)
/home/rydroid/workspace/check/tests/check_check_sub.c:2238: error: Line length exceed 277 (max=130)
/home/rydroid/workspace/check/tests/check_check_sub.c:2247: error: Line length exceed 281 (max=130)
/home/rydroid/workspace/check/tests/check_check_sub.c:2248: error: Line length exceed 281 (max=130)
/home/rydroid/workspace/check/tests/check_check_sub.c:2669: error: function (make_sub_suite) has too many lines: 437 (150)
/home/rydroid/workspace/check/tests/test_output.sh:60: error: Line length exceed 169 (max=130)
/home/rydroid/workspace/check/tests/test_output.sh:61: error: Line length exceed 169 (max=130)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I then agree that, at least for now, the tests folder need not be checked. If you are interested in proposing fixes for the existing issues, feel free to have a lax configuration for now and it can be incrementally tightened as times goes on.

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

Successfully merging this pull request may close these issues.

1 participant