Skip to content

Remove AlertPatcher - #3134

Merged
hannesj merged 17 commits into
opentripplanner:dev-2.xfrom
CGI-Move:remove-alert-patcher
Aug 27, 2020
Merged

Remove AlertPatcher#3134
hannesj merged 17 commits into
opentripplanner:dev-2.xfrom
CGI-Move:remove-alert-patcher

Conversation

@hannesj

@hannesj hannesj commented Jul 7, 2020

Copy link
Copy Markdown
Contributor

There are some todos left in the SIRI-SX updater, as well as tests.

To be completed by pull request submitter:

To be completed by @opentripplanner/plc:

  • reviews and approvals by 2 members, ideally from different organizations
  • after merging: update the relevant card on the roadmap

@hannesj
hannesj requested a review from a team July 7, 2020 12:39
@hannesj
hannesj force-pushed the remove-alert-patcher branch from d92c000 to 13178d3 Compare July 8, 2020 11:59
@t2gran t2gran added OTP2 !Technical Debt Improve code quality, no functional changes. labels Jul 30, 2020
@t2gran t2gran self-assigned this Aug 6, 2020
@t2gran
t2gran self-requested a review August 6, 2020 09:36
t2gran
t2gran previously requested changes Aug 26, 2020

@t2gran t2gran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I general I think this looks good, and I am in favor of merging this and then fix any problems if any. @gmellemstrand or @lassetyr could make the approval after coordinating on reviewing the Siri updaters and testing it. At Entur we should patch the Transmodel API as soon as possible after the merge, since this is blocking us from merging it into our OTP2 production branch.

Comment thread src/main/java/org/opentripplanner/api/model/ApiAlert.java
Comment on lines +6 to 11
import org.opentripplanner.routing.alertpatch.AlertUrl;
import org.opentripplanner.routing.alertpatch.EntitySelector;
import org.opentripplanner.routing.alertpatch.StopCondition;
import org.opentripplanner.routing.alertpatch.TimePeriod;
import org.opentripplanner.routing.alertpatch.TransitAlert;
import org.opentripplanner.routing.core.TraverseMode;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lassetyr Would you mind to review this part? The changes to the Siri updaters.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I am also in favor of merging.

@hannesj
hannesj force-pushed the remove-alert-patcher branch from cf82a60 to 9b796fb Compare August 27, 2020 07:26
@hannesj
hannesj force-pushed the remove-alert-patcher branch from 9b796fb to 7402dae Compare August 27, 2020 07:28
@hannesj
hannesj requested a review from t2gran August 27, 2020 08:19

@abyrd abyrd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @t2gran, this looks good, we can merge and update later if we spot any problems. @lassetyr has confirmed he doesn't expect any conflicts if we merge this first, then he merges the result into the siri branch.

@hannesj
hannesj dismissed t2gran’s stale review August 27, 2020 08:42

Fixed all issues

@hannesj
hannesj merged commit ae19990 into opentripplanner:dev-2.x Aug 27, 2020
@hannesj
hannesj deleted the remove-alert-patcher branch August 27, 2020 08:42
lassetyr added a commit to entur/OpenTripPlanner-LegacyHSLFork that referenced this pull request Aug 27, 2020
@abyrd abyrd added this to the 2.0 milestone Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Technical Debt Improve code quality, no functional changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove AlertPatcher

4 participants