Closed elipavlov closed 3 years ago
@casualjim @fredbi , could you please look at this PR.
This PR uses binary implementation of ULID as the underlying (embedded) type. It works much faster and I decided that it would be more helpful to have this implementation out-of-the-box after unmarshalling a request. But tradeoff is to have one new dependency.
There is another implementation, which relies only on regexp and string representation: #79
Maybe it would be enough to leave regexp version and leave the rest to developers themselves.
Benchmark comparison for binary and regexp implementation was:
goos: darwin
goarch: amd64
pkg: github.com/go-openapi/strfmt
BenchmarkParseStrict-4 46144773 25.7 ns/op
BenchmarkRegexpIsULID-4 2117212 585 ns/op
PASS
ok github.com/go-openapi/strfmt 4.021s
PASS
ok github.com/go-openapi/strfmt/conv 0.127s
I'm going to add more tests to this PR if we choose it instead of #79.
it's fine to add a dependency. My vote goes to this one too
Merging #80 (e78627f) into master (a4e46ea) will increase coverage by
0.52%
. The diff coverage is90.36%
.
@@ Coverage Diff @@
## master #80 +/- ##
==========================================
+ Coverage 81.67% 82.20% +0.52%
==========================================
Files 10 12 +2
Lines 1288 1371 +83
==========================================
+ Hits 1052 1127 +75
- Misses 164 170 +6
- Partials 72 74 +2
Impacted Files | Coverage Δ | |
---|---|---|
ulid.go | 89.18% <89.18%> (ø) |
|
conv/ulid.go | 100.00% <100.00%> (ø) |
|
format.go | 75.17% <100.00%> (+0.88%) |
:arrow_up: |
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 a4e46ea...e78627f. Read the comment docs.
More details about format: https://github.com/ulid/spec
It uses binary underlying implementation: https://github.com/oklog/ulid
Resolves: #77