Conversation
|
Hi @ajozwik, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement: |
|
Can one of the admins verify this patch? |
|
I signed the agreement and add the test (21.sbt.txt) |
|
Can somebody restart the failed builds? The error log: |
|
One test fail - related to work-around with xml. Build was restarted after commit. |
|
command |
|
@eed3si9n @typesafehub-validator - the build failed on travis - but the problem was with system resource - not with tests. I ran manually |
|
Related to #1666 |
|
@eed3si9n @typesafehub-validator @jsuereth Can somebody of you restart the build https://travis-ci.org/sbt/sbt/jobs/37916032 ? |
|
I tried to simplify the code for xml content (and change the comments to the methods) - but I still do not have a time :) My (private) code still failed the tests - I will merge it in few days |
|
Thanks for taking care of the bug. Is this PR ready to be merged? |
|
Please take me two days. |
…dExplicitXmlContent use pattern matching, splitFile - does not return whitespace statements
|
If test pass - ready to merge |
|
The parallel builds on travis share the same directories. If the /tmp directory is not empty - the problem can occur. Locally I started with |
|
I ran (sequential): sbt clean "scripted compiler-project/*" > compiler.log || exit 1
sbt clean "scripted project/*2of2" > 2of2.log || exit 1
sbt clean "scripted tests/*" > tests.log || exit 1and all tests passed (tests which failed on travis). |
|
I restarted the failing Travis jobs. Could you open a new issue for flaky builds? |
|
The sbt builds are not deterministic. I did not reproduce the error. Now the build hung on |
|
Yeah, i'm still working on our own jenkins infrastructure where (for a lot less $$ than travis) we can get better hardware to run tests. For now, we'll deal with some flaky builds. BUT, the regression in performance/time is recent and, I think, something to investigate. |
There was a problem hiding this comment.
why are we removing all these from this file? Shouldn't the methods in EvaluateConfigurations delegate down to this deprecated guy?
There was a problem hiding this comment.
@gkossakowski will implement property/settings to revert old parser. So I think the old splitExpression will not be removed. Maybe it will be renamed/private. So I use original implementations.
There was a problem hiding this comment.
I believe it's @eed3si9n who opened a ticket about an ability to revert to old parsing logic and planned to work on a fix.
There was a problem hiding this comment.
@gkossakowski We already have that in place, AFAIK. This looks like we're removing the copy-pasted old version code. I'm ok with that, I guess, as the original implementation is deprecated, but why do we need this new object at all if we're going to decimate its methods? What the comment is about is how the original code should delgate to this one, instead of removing implementation in this object and delegate back to the original (deprecated) location.
In any case, let's merge this as-is and we can do any deprecation stuff on our end as needed.
There was a problem hiding this comment.
The link to falback: #1645 (comment) - but I do not know the plan/progress.
Improve xml handling
Fixed #1666. Previous parser does not work for multi line statements with
//