Closed adamdecaf closed 1 year ago
Base: 71.21% // Head: 71.30% // Increases project coverage by +0.08%
:tada:
Coverage data is based on head (
3940453
) compared to base (68aee25
). Patch coverage: 100.00% of modified lines in pull request are covered.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
This is a really good change to make code simpler for the callers
@alovak Cool. I can add this to every field type.
thanks @adamdecaf! this PR looks 💪
the only thing that (kind of) worries me is that GetValue. On message
we have GetString()
, GetBytes()
. But on field
we have String()
and Bytes()
and now we add GetValue()
.
We can introduce a breaking change by adding Value()
and SetValue()
and renaming Value
into value
. It will make code more consistent and safe. In my personal opinion, such change is just find/replace so it should not be a PITA.
thoughts?
Yea I agree that GetValue()
isn't idea and I'm okay with that breaking change. It's minor enough and increases runtime safety.
@alovak feel free to merge this whenever you want. I know there are other 8583 changes in flight.
Issue: https://github.com/moov-io/iso8583/issues/195