search

Dwolla / dwolla-v2-csharp

Official C# Wrapper for Dwolla's API
https://developers.dwolla.com
MIT License
12 stars 16 forks source link

Add High-level requestframework #55

Closed IsaiahDahlberg closed 3 months ago

IsaiahDahlberg commented 1 year ago

Here is a High-level request framework proof of concept.

I created a second ExampleApp that shows how to use the HttpServices

ShreyaThapa commented 1 year ago

Awesome! Thanks, @IsaiahDahlberg! 🙌

We've got this on our radar and will be reviewing it in our upcoming sprint. We'll be sure to keep you posted on any feedback we may have.

Once again, thank you for your valuable contribution! 🙏

IsaiahDahlberg commented 1 year ago

@ShreyaThapa, this PR has been updated with the last of the missing HttpServices and is ready for another review!

ShreyaThapa commented 1 year ago

Hi @IsaiahDahlberg!

🎉 First of all, a big thanks to you and @miccav01 for your contribution! We really appreciate your effort and the time you've put into this pull request.

Right now, our team is heads down, focusing on initiatives with our new product - Dwolla Connect. As a result, our review process might experience some delay. Rest assured, your pull request is important to us and we will definitely get to it in our upcoming sprints.

Thanks again for your patience and understanding. We're excited to dive into your code soon!

ShreyaThapa commented 9 months ago

Hi @IsaiahDahlberg and @miccav01!

🎉 Thank you once again for your valuable contribution to our project! We've thrilled to finally have had a chance to prioritize this PR and have reviewed it. Your effort is greatly appreciated.

While reviewing, we've noticed a few minor issues, which we've highlighted in the comments above along with some suggested quick fixes. Also, it seems the test file isn't passing smoothly. Did you encounter any challenges running it? Perhaps it might need an update to accommodate the new HTTP Services structure.

Additionally, we have made note of some additional schemas and models to add and update. We're in the process of creating tickets on our board to address these, aligning with our high-level methods documentation and the updated OpenAPI spec.

Regarding the release plan for this change, here's what we were planning: Once approved, we'll merge these updates along with any changes Dwolla makes related to high-level methods into an intermediary branch. This branch will be marked as a Beta pre-release and published to NuGet for developers to integrate into their applications. We'll be actively collecting feedback during this phase to ensure everything runs smoothly. Once we've gathered sufficient usage and feedback, we'll proceed with the main release.

We'd love to hear your thoughts on this plan!

Thank you again for your patience and understanding throughout this process. Your efforts are truly appreciated! 🙌

isaiah-eventlink commented 7 months ago

@ShreyaThapa, that plans sounds good.

I resolved all the comments. This PR should be ready for another review. Although, I wasn't able to fully test any Label functionality as its behind a pay wall. But, those tasks should be working fine as it seemed the majority of the issues were all related.

ShreyaThapa commented 7 months ago

Thanks for the new commits addressing our comments, @isaiah-eventlink! 🙌 We'll take a look and make sure to test the labels endpoints as well!

natehitze-eventlink commented 5 months ago

@ShreyaThapa Any idea on a timeline for merge or review? We're going to need this soon so we're trying to make a decision on whether to use a private fork or not.

ShreyaThapa commented 4 months ago

Hi @natehitze-eventlink! Thank you for bringing this to our attention. We've pulled this into our current sprint, which is scheduled to end this month. We will re-review for merging and releasing this branch as a beta release on NuGet for you to import and use in your app!

ShreyaThapa commented 3 months ago

Hi @natehitze-eventlink and @isaiah-eventlink!

Thank you for addressing the comments I made during my last review! I appreciate the effort you put into resolving them.

I am currently running into some issues when trying to pass the tests using dotnet test. It’s possible that I might be doing something wrong on my end. Could you provide some guidance on how to properly run the tests? Once I can get the tests to pass, I will be happy to approve the PR and publish it!

Thanks again for your contribution.

isaiah-eventlink commented 3 months ago

@ShreyaThapa, the tests were missing a callback. I don't recall how they ended up that way. Anyawho, the tests should now be passing for you

ShreyaThapa commented 3 months ago

Awesome! Thanks for the quick update, @isaiah-eventlink!

I have confirmed that the tests are running and passing for me. I've created a new branch v7.0.0-alpha for all changes related to high level methods. Would you mind re-opening the PR against that branch? After that, I can go ahead and approve/merge this branch, and work on publishing it to NuGet.

isaiah-eventlink commented 3 months ago

@ShreyaThapa, here is the second PR you requested https://github.com/Dwolla/dwolla-v2-csharp/pull/58

ShreyaThapa commented 3 months ago

Perfect! Thank you @isaiah-eventlink!

I've merged your branch to the new v7.0.0-alpha branch. I'll close out this PR. Will let you know when it's published! 🙌