Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetTraceData is accepting int instead of long? #2

Open
HarelM opened this issue Oct 19, 2019 · 6 comments
Open

GetTraceData is accepting int instead of long? #2

HarelM opened this issue Oct 19, 2019 · 6 comments

Comments

@HarelM
Copy link
Collaborator

HarelM commented Oct 19, 2019

GetTraceData is accepting int, shouldn't it accept long like the rest of the API?

@HarelM
Copy link
Collaborator Author

HarelM commented Oct 21, 2019

@blackboxlogic any thoughts? I don't mind replacing the code from int to long but you might know something I don't...?

@blackboxlogic
Copy link
Collaborator

blackboxlogic commented Oct 21, 2019

OsmSharp.API.GpxFile.Id is an int and I followed their lead without wondering if it was right. The API documentation isn't specific. I've just asked on dev channel in slack, waiting for an asnwer.

I foresee problems if we use long and OsmSharp.core uses and int. Probably best to try to fix both if you want to make the change.

@HarelM
Copy link
Collaborator Author

HarelM commented Oct 21, 2019

I see. Well, we'll need to do it in two steps - OsmSharp.core first and here later which will require two NuGet releases... :-/
If I needed to guess since this is a database key it's probably long. I'd be happy if you can take this one since you changed a lot of the classes in OsmSharp.core in the previous release.

@blackboxlogic
Copy link
Collaborator

Confirmed in slack by someone that sounded confident: it's a long. If there are no traces with large IDs, then you don't need to change OsmSharp.core. That would just be bonus points.

@blackboxlogic
Copy link
Collaborator

blackboxlogic commented Oct 22, 2019

I'm noticing that element versions should also be long instead of int according to the db schema.

OsmSharp.OsmGeo.Version is currently an int? and should be a long?

Maybe worth fixing at the same time.

@blackboxlogic
Copy link
Collaborator

I've made OsmSharp/core#89 for the OsmSharp side of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants