12:52 p.m.
Hey Guys,
Looking like some test failures were introduced in the faces-scaffold
project due to the new JavaParser formatting.
Ivan, could you or someone else take a look if you have a free moment?
Otherwise I'll keep working on them as I get a chance. It looks like purely
a formatting issue, not a functional one. So the tests just need to be
updated.
Mucho gracias :)
--
Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."
7:26 p.m.
as Lincoln already mentioned the problem is mainly a formating issue.
There are to sides to the problem:
- pure indentation changes: \t to " "
- change in positioning of braces: Java style to C# style
The Code Conventions for Java Programming Language is very clear about
that, so this is kinda question of taste.
One possibility would be to normalize all whitespace like
content.replaceAll("(\r|\n|\\s)+"," ")
but then the idea of the test would become somewhat absurd, because this
is, what the test should test.
And then it would _not_ solve the braces positioning issue :-/
Thomas
7:36 p.m.
Well, in this case I think that's acceptable because this test is not
really intended to test brace positioning. This is to test generation
functionality.
On Tue, May 22, 2012 at 7:26 PM, Thomas Frühbeck <fruehbeck(a)aon.at> wrote:
as Lincoln already mentioned the problem is mainly a formating
issue.
There are to sides to the problem:
- pure indentation changes: \t to " "
- change in positioning of braces: Java style to C# style
The Code Conventions for Java Programming Language is very clear about
that, so this is kinda question of taste.
One possibility would be to normalize all whitespace like
content.replaceAll("(\r|\n|\\s)+"," ")
but then the idea of the test would become somewhat absurd, because this
is, what the test should test.
And then it would _not_ solve the braces positioning issue :-/
Thomas
_______________________________________________
forge-dev mailing list
forge-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/forge-dev
--
Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."
2:33 a.m.
Hi folks,
I haven't got the chance to look at the issue, but it looks to me that the
test should be changed to support the fact that we already format our java
classes?
Cheers,
Ivan
On Wed, May 23, 2012 at 2:36 AM, Lincoln Baxter, III <
lincolnbaxter(a)gmail.com> wrote:
Well, in this case I think that's acceptable because this test is
not
really intended to test brace positioning. This is to test generation
functionality.
On Tue, May 22, 2012 at 7:26 PM, Thomas Frühbeck <fruehbeck(a)aon.at> wrote:
> as Lincoln already mentioned the problem is mainly a formating issue.
> There are to sides to the problem:
> - pure indentation changes: \t to " "
> - change in positioning of braces: Java style to C# style
> The Code Conventions for Java Programming Language is very clear about
> that, so this is kinda question of taste.
>
> One possibility would be to normalize all whitespace like
> content.replaceAll("(\r|\n|\\s)+"," ")
> but then the idea of the test would become somewhat absurd, because this
> is, what the test should test.
> And then it would _not_ solve the braces positioning issue :-/
>
> Thomas
> _______________________________________________
> forge-dev mailing list
> forge-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/forge-dev
>
--
Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."
_______________________________________________
forge-dev mailing list
forge-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/forge-dev
4:25 a.m.
I somewhat agree, I nearly finished a proposal for a fix.
But AFAIK one of the goals of the faces scaffold - in its role of Java
code generator - is to provide clean an correctly formatted code. So it
goes quite a distance to assure the correctness.
Now the formatting seems to happen in between, and the config of the
formatter is different from the Java templates of faces scaffold.
My fix would "normalize" this by simply compressing white space. And so
render the indentation aware tests useless.
I will send the fix ASAP, and let the maintainers decide what to do :-)
Thomas
4:38 a.m.
OK, here's my pull request: https://github.com/forge/core/pull/158
It does what it says with the IMHO the least possible loss of test quality.
Thomas
Am 23.05.2012 01:36, schrieb Lincoln Baxter, III:
Well, in this case I think that's acceptable because this test is
not
really intended to test brace positioning. This is to test generation
functionality.
4354
days inactive
4355
days old
5 comments
3 participants
participants (3)
-
Ivan St. Ivanov -
Lincoln Baxter, III -
Thomas Frühbeck