some refactoring #1629
some refactoring #1629
Conversation
|
Formatting check succeeded! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1629 +/- ##
============================================
+ Coverage 65.63% 65.76% +0.13%
Complexity 1646 1646
============================================
Files 473 472 -1
Lines 27485 27426 -59
Branches 5475 5457 -18
============================================
- Hits 18039 18037 -2
+ Misses 7141 7090 -51
+ Partials 2305 2299 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| private set | ||
|
|
||
| var compiledLibrary: CompiledLibrary? = null | ||
| var root: Any? = null |
There was a problem hiding this comment.
@JPercival This is set to the result of Cql2ElmVisitor.visit() inside of run() and contains the last define from the input CQL content. Might be helpful to keep the original visitResult? I could alternatively remove it because we only use it in tests, where instead we could explicitly look for the last ExpressionDef in the compiledLibrary. I think this would be better.
There was a problem hiding this comment.
I like your suggestion. I had a similar thought, I just didn't chase it far enough to reach your same conclusion.
| libraryManager.compiledLibraries[lib.identifier!!] = compiler.compiledLibrary!! | ||
|
|
||
| if (!compiler.errors.isEmpty()) { | ||
| if (!compiler.exceptions.isEmpty()) { |
There was a problem hiding this comment.
This used to look only at errors, so should be
if (compiler.exceptions.any { it.severity == CqlCompilerException.ErrorSeverity.Error }) { ... }and only use the errors below
| val library = compiler.run(inputStream!!.asSource().buffered()) | ||
|
|
||
| if (!compiler.errors.isEmpty()) { | ||
| if (!compiler.exceptions.isEmpty()) { |
There was a problem hiding this comment.
I can change this also to only look at errors as before.
|
No description provided.