Adds general options and missing options for table request

[mkasner]

No description provided.

[codecov-io]

Codecov Report

Merging #13 into master will increase coverage by 0.27%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
+ Coverage   96.55%   96.82%   +0.27%     
==========================================
  Files           8        8              
  Lines         232      252      +20     
==========================================
+ Hits          224      244      +20     
  Misses          4        4              
  Partials        4        4

Impacted Files	Coverage Δ
nearest.go	100% <100%> (ø)	⬆️
match.go	100% <100%> (ø)	⬆️
types.go	96.55% <100%> (+0.25%)	⬆️
route.go	100% <100%> (ø)	⬆️
options.go	100% <100%> (ø)	⬆️
table.go	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 788b384...7fc54cc. Read the comment docs.

[mkasner]

Updated code with proposed changes

[regeda]

First of all, great job to make types reusable in different requests.
I have a question: how to be with backward compatibility?
My code does the following:

req := osrm.MatchRequest{
   Hints: []string{"X","Y"},
}

but new change will break compilation process because I need to change the code:

req := osrm.MatchRequest{
    GeneralOptions: osrm.GeneralOptions{
         Hints: []string{"X","Y"},
    },
}

[regeda]

FallbackCoordinateSnapped FallbackCoordinate as well

[mkasner]

Yes, you are right.
I used this like iota :)

[regeda]

It doesn't work like iota. You must declare the type explicitly.
https://play.golang.org/p/iVeaitEtA2k

[regeda]

Why do you need that?
Maybe add a function to combine multiple annotations in a request?

[mkasner]

I can do the function. There's a request in TabeService which combines duration and distance, and returns it in the response. That's the reason I need it.
I thought this would be more simpler, since this is considered like a 3rd option for that param in request.

[regeda]

I think you could add a function:

func (a Annotations) Concat(b Annotations) Annotations {
    return a + "," + b
}

and use it in your code as well:

AnnotationsDuration.Concat(AnnotationsDistance)

[mkasner]

I was thinking, if I add a function which enables to combine multiple annotations, then users of the library can product invalid annotations which cannot be combined.
By defining it like this user is always going to produce valid annotations.
This 'duration,distance' is the only combination in the project osrm docs.
I would create a function, if there were more combinations possible, but since it's only one, I would rather leave it defined as const.

[mkasner]

First of all, great job to make types reusable in different requests.
I have a question: how to be with backward compatibility?
My code does the following:

req := osrm.MatchRequest{
   Hints: []string{"X","Y"},
}

but new change will break compilation process because I need to change the code:

req := osrm.MatchRequest{
    GeneralOptions: osrm.GeneralOptions{
         Hints: []string{"X","Y"},
    },
}

Yes, this cannot be backward compatible, unless I copy params from the embedded struct GeneralOptions to every request.
I thought this was cleaner approach from the standpoint of library, but it is a breaking change in terms of clients using this library.
You as a reviewer, must be the judge of that.
If you don't want to introduce breaking change, then don't approve this PR, so I can create another one with repeated fields in every request.

[regeda]

First of all, great job to make types reusable in different requests.
I have a question: how to be with backward compatibility?
My code does the following:

req := osrm.MatchRequest{
   Hints: []string{"X","Y"},
}

but new change will break compilation process because I need to change the code:

req := osrm.MatchRequest{
    GeneralOptions: osrm.GeneralOptions{
         Hints: []string{"X","Y"},
    },
}

Yes, this cannot be backward compatible, unless I copy params from the embedded struct GeneralOptions to every request.
I thought this was cleaner approach from the standpoint of library, but it is a breaking change in terms of clients using this library.
You as a reviewer, must be the judge of that.
If you don't want to introduce breaking change, then don't approve this PR, so I can create another one with repeated fields in every request.

I like that PR. But I think maintainers must release a major version of it (for example, v0.2.0) with a notice about the backward-incompatible change.

[mkasner]

that PR. But I think maintainers must release a major version of it (for example, v0.2.0) with a notice about the backward-incompatible change.

I didn't yet added changes you requested, about constants type naming, and function for combining duration and distance

[regeda]

It doesn't work like iota. You must declare the type explicitly.
https://play.golang.org/p/iVeaitEtA2k

[mkasner]

Sure, I know that.
I said, that I used it like iota by mistake.
I'm going to push that change now.

[regeda]

I think you could add a function:

func (a Annotations) Concat(b Annotations) Annotations {
    return a + "," + b
}

and use it in your code as well:

AnnotationsDuration.Concat(AnnotationsDistance)