abrensch / brouter

configurable OSM offline router with elevation awareness, Java + Android
MIT License
494 stars 118 forks source link

Prevent breaks on future update on version number #595

Closed afischerdev closed 1 year ago

afischerdev commented 1 year ago

This update aims to avoid breaks in static functions when merging PR #457.

rkflx commented 1 year ago

For anyone wondering what this is about (given the sparse description and ambiguous title) and wanting to save some time scratching their head:

It's basically a compile fix for rebasing #457 on master. 3c5ac66 recently added static functions with read access to the version variable. However, #457 will change version to be a function accessing the non-static method getClass(), so the newly added functions won't compile anymore unless they are made non-static too, like some other functions in this class.

That being said, I'd solve the issue differently. Using the following patch for #457 should work too, with less overhead and being more broadly applicable (i.e. also usable in static functions):

public final class OsmTrack {
-  final private String version = getClass().getPackage().getImplementationVersion();
+  final private static String version = OsmTrack.class.getPackage().getImplementationVersion();
afischerdev commented 1 year ago

For anyone wondering what this is about (given the sparse description and ambiguous title) and wanting to save some time scratching their head:

Please look after your words.

There are always more options. But I agree your way is more elegant and I shouldn't have done anything. So I can close this and let others play the ball.

rkflx commented 1 year ago

I'm sorry if you felt the wording being too blunt. My intention was not to pick on you personally, but to point out why someone other than the author might have trouble understanding some of the PR. If your are not interested in this kind of feedback, that's okay. I guess I should have worded it as yet another "What do you mean by that?"-inquiry too common in this project.

At least I initially took a while to understand this is not about a breakdown once a version number will be increased, and why removing static would solve anything. If other readers of the PR are more clever, good for them.

Anyway, good luck finding other reviewers for your PRs, I won't bother anymore for the most part.

afischerdev commented 1 year ago

@rkflx I'm sorry, I missed your statement while closing. The feedback is not the problem, support, quality assurance, improvements, ... are welcome and needed. The thing is the style of communication, there are different ways of presentation.

rkflx commented 1 year ago

My focus will be elsewhere going forward, sorry for the noise.