Can you give us a number, like how many checkstyle inconsistencies are there?  If only a dozen, then maybe we can suffer a bit now and have people who modify files also fix the style in those files.  If hundreds, then we can probably just keep it as is, and maybe only add the copyright note; but in that case, are we doing anything to make sure that all code passes the checkstyle?

Grigore


________________________________
From: dwightguth [notifications@github.com]
Sent: Friday, April 11, 2014 9:38 AM
To: kframework/k
Cc: Rosu, Grigore
Subject: Re: [k] add regular expression pattern matching for ktest (#356)


In src/javasources/KTool/src/org/kframework/ktest/RegexStringMatcher.java:

> +        boolean matches = true;
> +        for (String pattern : patterns.split("\n")) {
> +            Matcher m = Pattern.compile(pattern).matcher(actual);
> +            matches &= m.find();
> +            if (!matches) {
> +                failedRegex = pattern;
> +                failedActual = actual;
> +                break;
> +            }
> +        }
> +        return matches;
> +    }
> +
> +    @Override
> +    public String errorMessage() {
> +        return String.format("Expected result to match regular expression:%n%s%n%nbut found:%n%s%n", failedRegex, failedActual);


Since we already have code that runs checkstyle on the code modified by a diff, this would be easy to set up. The only problem is it would require users to fix every line in a file when they modify it, and not just their own lines. That would be kind of intrusive. Making it only detect modified /lines/ would be a lot harder to do and require more effort. How do you want me to spend my time?
…
On Apr 11, 2014 7:43 AM, "Grigore Rosu" <notifications@github.com> wrote: In src/javasources/KTool/src/org/kframework/ktest/RegexStringMatcher.java: > + boolean matches = true; > + for (String pattern : patterns.split("\n")) { > + Matcher m = Pattern.compile(pattern).matcher(actual); > + matches &= m.find(); > + if (!matches) { > + failedRegex = pattern; > + failedActual = actual; > + break; > + } > + } > + return matches; > + } > + > + @Override<https://github.com/Override> > + public String errorMessage() { > + return String.format("Expected result to match regular expression:%n%s%n%nbut found:%n%s%n", failedRegex, failedActual); Dwight, would it make sense to incorporate a check for line length in the checkstyle script? Also, would it make sense to send a "style violation warning" message when the style is violated, so people learn to not do it anymore? Thanks, Grigore ________________________________ From: Ömer Sinan Aðacan [ notifications@github.com] Sent: Thursday, April 10, 2014 2:03 PM To: kframework/k Cc: Rosu, Grigore Subject: Re: [k] add regular expression pattern matching for ktest (#356<https://github.com/kframework/k/pull/356><https://github.com/kframework/k/pull/356>) In src/javasources/KTool/src/org/kframework/ktest/RegexStringMatcher.java: + boolean matches = true; + for (String pattern : patterns.split("\n")) { + Matcher m = Pattern.compile(pattern).matcher(actual); + matches &= m.find(); + if (!matches) { + failedRegex = pattern; + failedActual = actual; + break; + } + } + return matches; + } + + @Override<https://github.com/Override><https://github.com/Override>+ public String errorMessage() { + return String.format("Expected result to match regular expression:%n%s%n%nbut found:%n%s%n", failedRegex, failedActual); This line is 128 chars long... this is not compliant to our coding conventions. -- Reply to this email directly or view it on GitHub< https://github.com/kframework/k/pull/356/files#r11501532>. -- Reply to this email directly or view it on GitHub<https://github.com/kframework/k/pull/356/files#r11530014> .

—
Reply to this email directly or view it on GitHub<https://github.com/kframework/k/pull/356/files#r11534900>.