-
Notifications
You must be signed in to change notification settings - Fork 211
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Description> | ||
<!-- | ||
Copying and distribution of this file, with or without modification, | ||
are permitted in any medium without royalty provided this notice is | ||
preserved. This file is offered as-is, without any warranty. | ||
Names of contributors must not be used to endorse or promote products | ||
derived from this file without specific prior written permission. | ||
--> | ||
|
||
<!-- Text --> | ||
<LineLength>130</LineLength> <!-- max --> | ||
<EmptyLines>2</EmptyLines> <!-- max --> | ||
<EndOfFileNewLine>true</EndOfFileNewLine> | ||
|
||
<!-- C/C++ --> | ||
<VariablePerLine>3</VariablePerLine> <!-- max --> | ||
<StatementPerLine>1</StatementPerLine> <!-- max --> | ||
<Functions> | ||
<regex>[a-z_]</regex> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this ensure that function names There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
<length>150</length> | ||
</Functions> <!-- length = number of lines --> | ||
<Comments> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This prevents comments that start with There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
<begin>/*</begin> | ||
<middle> * </middle> | ||
<end> */</end> | ||
<checkMissingComment>false</checkMissingComment> | ||
</Comments> | ||
</Description> |
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.
Why not also test the
tests
directory?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.
I could not figure out from the documentation what
-gcc
does. What does it do?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.
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.
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.
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.
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.
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.
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.