Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Nil nsid desperate fix #19

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Nil nsid desperate fix #19

wants to merge 3 commits into from

Conversation

dedsm
Copy link

@dedsm dedsm commented Nov 24, 2014

this tries to fix #18

if I see the nsid is nil, I try to find the element in the xmlns namespace.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f504bc4 on YACFirm:nil_nsid into 95375e8 on savonrb:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7fdc7fe on YACFirm:nil_nsid into 95375e8 on savonrb:master.

@tjarratt
Copy link
Contributor

tjarratt commented Dec 2, 2014

Hey @dedsm I appreciate you submitting this pull request, but I'm hesitant to merge anything into Sekken without any tests for the functionality. Since Sekken is still fairly new, it would be helpful to have tests that express the desired behavior so that any refactoring that is done later can be done safely, knowing that there are tests for all of the existing features.

Is that something you'd be interested in doing?

@dedsm
Copy link
Author

dedsm commented Dec 2, 2014

sure, right now I'm on vacations but from next week I can start working on
this, I didn't do it because I don't know if my case is a valid one because
my experience with xml is near zero, so I hoped you guys to look into it
and see if this is the best way of handling this.

El mar, 2 de diciembre de 2014 03:28, Tim Jarratt notifications@github.com
escribió:

Hey @dedsm https://github.com/dedsm I appreciate you submitting this
pull request, but I'm hesitant to merge anything into Sekken without any
tests for the functionality. Since Sekken is still fairly new, it would be
helpful to have tests that express the desired behavior so that any
refactoring that is done later can be done safely, knowing that there are
tests for all of the existing features.

Is that something you'd be interested in doing?


Reply to this email directly or view it on GitHub
#19 (comment).

@tjarratt
Copy link
Contributor

tjarratt commented Dec 3, 2014

Thanks for the honesty 😀. Unfortunately, I've never seen this, and I doubt many other people have because Sekken is still relatively new. Most of the features in Savon and Sekken have come about because someone found that the gem did not work with their SOAP service, so they extended. That's one of the best parts of open source software in my opinion; you get a chance to improve it too! We might be experts at some parts of ruby and SOAP, but we don't know everything (in particular we have no idea how well Sekken works with your API).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

with a nil nsd, sekken fails to collect child elements
3 participants