search

danger / swift

⚠️ Stop saying "you forgot to …" in code review
https://danger.systems/swift/
MIT License
1.06k stars 141 forks source link

[Question] Why changesCount of gitlab merge request is String ? #338

Open amadeu01 opened 4 years ago

amadeu01 commented 4 years ago

The danger.gitLab.mergeRequest.changesCount shouldn't be an Int ? or There are cases where it comes with string value?

amadeu01 commented 4 years ago

The answer https://docs.gitlab.com/ee/api/merge_requests.html#get-single-mr

orta commented 4 years ago

Yep, cool - sounds like it needs changing 👍

I assume if it's working so far it'll need to handle both possible types somehow in the type system

f-meloni commented 4 years ago

I think it has to be a String, because it’s value can be “1000+”, and that can be expressed only with a String. We could try to convert it to an Int?, but that would potentially lead to a loss of information. Also I would like to keep it similar to danger-js where is a String, and to the actual API response. (And avoid a major change). Maybe we could have a getter that tries to convert it in addition to the string value we currently have?