Skip to content
This repository has been archived by the owner on Sep 30, 2019. It is now read-only.

Fix cfRef Resolver #113

Merged
merged 4 commits into from
Apr 30, 2019
Merged

Fix cfRef Resolver #113

merged 4 commits into from
Apr 30, 2019

Conversation

bboure
Copy link
Contributor

@bboure bboure commented Apr 24, 2019

@lightsofapollo as discussed in #112
This fixes my issue

@pianomansam
Copy link
Contributor

I've confirmed that using this fix on the fix-Ref-resolver branch also resolves the issue for me.

@cbaron
Copy link
Contributor

cbaron commented Apr 24, 2019

I don't have much context on this issue, but LGTM.

@bboure
Copy link
Contributor Author

bboure commented Apr 26, 2019

I had more issue because of the other Exceptions
I think I would remove them all and just return value in those cases.

@lightsofapollo
Copy link
Contributor

Nice thank you for the fix

@lightsofapollo
Copy link
Contributor

@bboure Can you update the test checking for this and potentially add a test with the case that effected you? I want to make sure we do not regress this behavior.

Copy link
Contributor

@lightsofapollo lightsofapollo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor ask and suggestion.

@bboure
Copy link
Contributor Author

bboure commented Apr 26, 2019

@lightsofapollo I added test based on my use case.

@lightsofapollo
Copy link
Contributor

@bboure Looks good to me! Ship it!

@bboure bboure merged commit 733b9ab into master Apr 30, 2019
@bboure bboure deleted the fix-Ref-resolver branch April 30, 2019 20:23
@bboure
Copy link
Contributor Author

bboure commented Apr 30, 2019

merged and version bumped.

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.

None yet

4 participants