search

moov-io / iso8583

A golang implementation to marshal and unmarshal iso8583 message.
https://moov.io
Apache License 2.0
304 stars 100 forks source link

Deprecate SetData method #288

Closed mfdeveloper508 closed 9 months ago

mfdeveloper508 commented 9 months ago

https://github.com/moov-io/iso8583/issues/278

codecov-commenter commented 9 months ago

Codecov Report

Attention: 23 lines in your changes are missing coverage. Please review.

Comparison is base (b05481c) 73.95% compared to head (05bccd5) 73.43%.

:exclamation: Current head 05bccd5 differs from pull request most recent head be33dce. Consider uploading reports for the commit be33dce to get more accurate results

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #288 +/- ## ========================================== - Coverage 73.95% 73.43% -0.52% ========================================== Files 43 43 Lines 2338 2338 ========================================== - Hits 1729 1717 -12 - Misses 378 390 +12 Partials 231 231 ``` | [Files](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io) | Coverage Δ | | |---|---|---| | [message.go](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io#diff-bWVzc2FnZS5nbw==) | `74.10% <ø> (-0.80%)` | :arrow_down: | | [field/bitmap.go](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io#diff-ZmllbGQvYml0bWFwLmdv) | `69.29% <60.00%> (-1.76%)` | :arrow_down: | | [field/binary.go](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io#diff-ZmllbGQvYmluYXJ5Lmdv) | `60.67% <40.00%> (ø)` | | | [field/hex.go](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io#diff-ZmllbGQvaGV4Lmdv) | `66.29% <40.00%> (-2.25%)` | :arrow_down: | | [field/numeric.go](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io#diff-ZmllbGQvbnVtZXJpYy5nbw==) | `80.89% <40.00%> (-2.25%)` | :arrow_down: | | [field/string.go](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io#diff-ZmllbGQvc3RyaW5nLmdv) | `80.72% <40.00%> (-2.41%)` | :arrow_down: | | [field/track1.go](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io#diff-ZmllbGQvdHJhY2sxLmdv) | `69.02% <40.00%> (ø)` | | | [field/track2.go](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io#diff-ZmllbGQvdHJhY2syLmdv) | `68.22% <40.00%> (ø)` | | | [field/track3.go](https://app.codecov.io/gh/moov-io/iso8583/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io#diff-ZmllbGQvdHJhY2szLmdv) | `61.79% <40.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/moov-io/iso8583/pull/288/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alovak commented 9 months ago

@mfdeveloper508 thanks for breaking down your PR into smaller ones. For this PR the summary of my review is:

  1. we should remove data field from all fields such as Numeric, String, etc. Please note, that in Binary field, it should stay or be renamed into value as it’s named in all fields.
  2. we should update some tests as after we remove data - I believe ~2-3 tests will fail. Just simplify them to keep tests only for Marshal and Unmarshal methods (setting/getting data in/from the passed arg)