deg0nz / MMM-PublicTransportBerlin

MagicMirror module to display public transport in Berlin and Brandenburg with BVG Hafas data.
MIT License
40 stars 20 forks source link

Update dependencies and a little fine tuning #135

Closed KristjanESPERANTO closed 1 year ago

KristjanESPERANTO commented 1 year ago

The update to hafas-client 6.x and stylelint 15.x made some changes necessary. I took the opportunity to optimze a few other details.

hafas-client 6.x requires at least node 16 and MagicMirror will drop support for lower versions at the end of April. I think that we can already take the step to switch to hafas-client v6.x now. I did that with MMM-PublicTransportHafas in December and didn't get any negative feedback.

rejas commented 1 year ago

WOuld be ok to merge that if @deg0nz agrees with the node change (and when the conflicts are resolved).

deg0nz commented 1 year ago

Alright. Since @KristjanESPERANTO has no bad experiences with the node bump, I'm all for it!

KristjanESPERANTO commented 1 year ago

I ran into a problem that I just noticed! I think it would be better if we discuss this before we merge the PR.

We have previously used stationIDs with 12 digits (e.g. "900000100003"). The hafas-client v6 can no longer cope with this. This doesn't seem to be an error, because I find the following entry in the changelog: "don't convert 7/9 <-> 12 digit IDs".

I changed the description of how to get the proper stationIDs (e.g. "900100003"). So there would be no problem for new users. But how can we properly notify "old" users that they need to update their stationIDs?

Or do you see a different approach?

deg0nz commented 1 year ago

I changed the description of how to get the proper stationIDs (e.g. "900100003"). So there would be no problem for new users. But how can we properly notify "old" users that they need to update their stationIDs?

Usually, I did this by emitting warnings in the logs some versions before the change happens and tried to add some compatibility layer. But since we have such a breaking change that doesn't make a compat layer possible, I suggest we should bump the version to 2.x and do the following:

(One could also build a stationID converter using both of the hafas versions, but I think this would be too much. Since a major version bump indicates breaking changes, this would be fine imho)

Edit: fix typos

KristjanESPERANTO commented 1 year ago

Okay, those are some good points! I'll try to work on that in the near future.

deg0nz commented 1 year ago

Yeah, no biggie if it takes time. I appreciate your (and @rejas') work so much! I would do it myself, but real life won't provide time to work in this until around mid of April for me. If it's still open until then, I'm happy to work on this :)

KristjanESPERANTO commented 1 year ago

I created a new PR (#141). It's almost the same as this one, but without the upgrade to the new hafas-client version and no breaking change.

In preparation for the upgrade, I have already changed the description how to determine the stationId. Luckily, hafas-client v5 can also handle the new (short) IDs. We can expand this later, but I think we could merge the new PR right away.