Conversation
…transit package and all depende
evansiroky
left a comment
evansiroky
left a comment
There was a problem hiding this comment.
Approved with the assumption that the failing tests are resolved in #387.
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
LGTM. Should com.conveyal.datatools.editor.utils.JacksonSerializers.java be deleted in favor of/merged into com.conveyal.gtfs.util.json.JacksonSerializers @evansiroky @landonreed?
src/main/java/com/conveyal/datatools/editor/utils/JacksonSerializers.java
Outdated
Show resolved
Hide resolved
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
After closer look, I recommend merging the two classes as mentioned in my comment #388 (review).
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
Approval still stands with one refactor comment.
|
|
||
| @Override | ||
| public LocalDate deserialize(JsonParser jp, DeserializationContext arg1) throws IOException { | ||
| LocalDate date; |
There was a problem hiding this comment.
Remove this declaration and inline all instances of date.
landonreed
left a comment
landonreed
left a comment
There was a problem hiding this comment.
Looks good. I would perhaps have marked the package removal commit with a BREAKING CHANGE note, but that functionality is long since unsupported and only used for converting old MapDB databases to Postgres (so perhaps it's not useful to mark it as such).
In any case, let's be sure to mark a breaking change in #387 when we update gtfs-lib to the latest release.
4 participants
Checklist
devbefore they can be merged tomaster)Description
This PR seeks to remove the now redundant transit package. This includes all containing classes and all depended classes and methods.
Note
The unit tests
MergeFeedsJobTest.canMergeRegionalandMergeFeedsJobTest.canMergeBARTFeedsare failing the new conditional checks (fare_rules - ForeignRefExistsCheck) introduced as part of the GTFS-lib update. Both are failing with over 4500k ref checks errors! A decision on the best approach to resolve is to be decided. Potential options are: