Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/525 return findings for ContainElement #527

Merged
merged 2 commits into from Mar 25, 2022

Conversation

thediveo
Copy link
Collaborator

@thediveo thediveo commented Mar 16, 2022

implements RFC #525:

  • implements an optionalPointer parameter to ContainElement for returning matching contained element(s)
  • adds unit tests related to returning matching contained elements
  • updates documentation for ContainElement

@thediveo thediveo changed the title Feature/525 return findings Feature/525 return findings for ContainElement Mar 16, 2022
@thediveo
Copy link
Collaborator Author

thediveo commented Mar 17, 2022

One note: the diff for the unit tests look ugly, but that's an unfortunate flaw of beloved diff. I've checked again and what I did was to wrap the existing tests for ContainElement in a Describe for the non-result-returning cases and then added separate new unit tests that cover the cases of returning macthed contained elements. More tests on the correct and useful error handling when the optional return pointer points at something that doesn't fit in with the type of actual.

@onsi
Copy link
Owner

onsi commented Mar 25, 2022

gah sorry again for the delay. I love this - it's gonna be super handy :)

@onsi onsi merged commit 1a4e27f into onsi:master Mar 25, 2022
4 checks passed
@onsi
Copy link
Owner

onsi commented Mar 25, 2022

hey @thediveo how would you feel about becoming a collaborator with commit rights? you're contributions have been super valuable and thoughtful!

No expectations on my end to do any maintenance/issues/PRs or anything like that (though if you're interested I won't say no 😉) - and I'm always happy to work on RFCs/PRs with you - but I don't want to be a blocker when my schedule is constrained.

@thediveo
Copy link
Collaborator Author

thediveo commented Mar 27, 2022

Hi @onsi I don't mind about the delays, they're part of our lives. I feel very honored by your offer and gladly accept, albeit I still want PR feedback and discussions, except for maybe some minor type fix in the documentation as I happen to come across them.

@thediveo thediveo deleted the feature/525-return-findings branch Mar 27, 2022
@onsi
Copy link
Owner

onsi commented Mar 27, 2022

🎉 sounds great! will add you now 😊

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

Successfully merging this pull request may close these issues.

None yet

2 participants