Decouple expression languages from core infrastructure #6502
Conversation
wetneb
force-pushed
the
grel_decoupling
branch
6 times, most recently
from
b1a68b3
to
64a638e
Compare
|
@tfmorris any interest in this? I'd like to merge it to be able to start the Maven modularization PR. |
...rver/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperationTests.java
Show resolved
Hide resolved
| storeCell(rows, rowindex, colindex, v); | ||
| Object date; | ||
| try { | ||
| date = CalendarParser.parseAsOffsetDateTime(val.get("date").asText()); |
There was a problem hiding this comment.
This also seems unrelated, but even if it gets moved to another PR, I don't think we should be introducing direct calls to CalendarParser which we should be targeting to get rid of. Who's date format is this? If it's something like an ISO date, we should just use the Java classes directly. If it's a human date, it should probably stay wrapped in ToDate()
There was a problem hiding this comment.
The motivation is still the same: removing a dependency from a core class to a GREL function. Using CalendarParser means that we support almost the same sort of date formats as before, which could be useful for services which rely on that.
But that being said now that the specs mandate ISO dates (although only in the JSON schemas so far, we should fix that) it should probably be safe to switch to supporting ISO dates only.
There was a problem hiding this comment.
Sorry for the delay in circling back on this. I think using Java classes to support ISO dates (only) would be a better approach.
Migrates the registration of expression languages provided by default to the controller.js. and removes references to GREL from functionally independent areas.
Migrates the registration of expression languages provided by default to the controller.js, and removes references to GREL from functionally independent areas.
This is a first step towards having GREL in its own Maven module.