search

railslove / cmxl

your friendly MT940 SWIFT file parser for bank statements
http://railslove.com
MIT License
46 stars 25 forks source link

allow defining parsers for non-digit tags #26

Closed prometh07 closed 5 years ago

bumi commented 5 years ago

@Uepsilon what do you think? for me this looks like a safe change. maybe we can add some specs for that parser?

bumi commented 5 years ago

which would probably be a spec to parse an example of such an MT940 file?

bumi commented 5 years ago

@Uepsilon did you merge it now without specs?!

Uepsilon commented 5 years ago

I did and added a spec afterwards.

On Fri, 25 Jan 2019, 23:07 Michael Bumann, notifications@github.com wrote:

@Uepsilon https://github.com/Uepsilon did you merge it now without specs?!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/railslove/cmxl/pull/26#issuecomment-457747831, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbobeV5EbD9DfkrCaSE96qw1qaS5xRtks5vG4AtgaJpZM4aDeCU .

bumi commented 5 years ago

ah, missed that. great! :tada:

Uepsilon commented 5 years ago

i wasn't sure if i could have appended my specs to the PR of @prometh07 as it was coming from his fork. is that possible or what would have been the way to go here?

bumi commented 5 years ago

yep, pushing in the branch and thus updating the PR would be best I guess.

Uepsilon commented 5 years ago

but i could not have pushed in that branch as it came from a fork i do not have write access to

Uepsilon commented 5 years ago

could have mirrored the branch and then updated the PR

prometh07 commented 5 years ago

I was going to add some specs but you guys were faster. ;)

Uepsilon commented 5 years ago

⚡️ 🏎 🏃 💨

:)

bumi commented 5 years ago

@Uepsilon typically (by default) it is enabled that maintainers have write access to the branch of the fork to update the PR. So you could have pushed to prometh07:nonswift_fields and this PR would have been updated. This is probably also a best practice as everything related to this PR is then documented here.

Uepsilon commented 5 years ago

ah. this is amazing to know. i was not aware of that.. thanks @bumi, i'll check it next time