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

feat: enable a new payment controller #40845

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

blaggacao
Copy link
Collaborator

@blaggacao blaggacao commented Apr 2, 2024

  • [ ... ] (5 commits are also part of preparatory PRs)


Here's a review branch without the preliminary code-shaping: blaggacao#1

no-docs

@blaggacao blaggacao force-pushed the chore/new-payments-interface branch 5 times, most recently from 53b1a4d to 3cbfb81 Compare April 3, 2024 00:33
@blaggacao blaggacao mentioned this pull request Apr 3, 2024
@blaggacao blaggacao force-pushed the chore/new-payments-interface branch 2 times, most recently from 2e79642 to 1330761 Compare April 3, 2024 11:33
Copy link

stale bot commented Apr 21, 2024

This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added inactive and removed inactive labels Apr 21, 2024
@blaggacao blaggacao force-pushed the chore/new-payments-interface branch 4 times, most recently from cdb0efa to 662e6a9 Compare May 2, 2024 19:45
@blaggacao blaggacao changed the title chore/new payments interface feat: enable a new payment controller May 3, 2024
@blaggacao blaggacao force-pushed the chore/new-payments-interface branch 2 times, most recently from 16a104f to 920743c Compare May 3, 2024 08:16
@blaggacao blaggacao marked this pull request as ready for review May 3, 2024 08:33
@blaggacao blaggacao force-pushed the chore/new-payments-interface branch from 920743c to 0a8c846 Compare May 3, 2024 08:33
@blaggacao
Copy link
Collaborator Author

blaggacao commented May 3, 2024

@s-aga-r From the standpoint of coordinating this PR in connection with it's dependent frappe/payments#53 this PR is fully backwards compatible and should be merged first in order to enable meaningful test execution on frappe/payments#53, at all.

Nonwithstanding amendments that will be required to this PR upon further insights gainer through future work on frappe/payments#53, this PR can be deemed at least "ready to review", if not "ready to merge (and iterate later)".

I appreciate your (hopeful) help and collaboration. Thank you in advance!

@blaggacao
Copy link
Collaborator Author

@s-aga-r Could you help with a commitment to engage on this one? My situation is that I want to contract with a well renown Frappe Partner for custom development and it's absolutely essential for us to be able to leverage the revised payment integration that this PR prepares on the ERPNext side. I promise, I'll leave you alone with my heavy handed contributions for a while after this. Sounds like a deal? 😃

@s-aga-r
Copy link
Contributor

s-aga-r commented May 23, 2024

@ruthra-kumar can you take this up?

@ruthra-kumar
Copy link
Member

ruthra-kumar commented May 24, 2024

@ruthra-kumar can you take this up?

As long as the changes are small and easy to reason about, I can help with review.

@blaggacao This PR is too big to work with. I would suggest breaking it into smaller, atomic changes.

@blaggacao
Copy link
Collaborator Author

ah! Ok, this explains all :-) Thanks for your comments in this case. It's marked "Pending" which means it's still in a sort of "staging" area and isn't visible to anyone else other than you. You'd need to click on "Finish your review" or something like that somewhere.

@blaggacao
Copy link
Collaborator Author

#42160 is necessary to effectively leverage frappe/frappe#25952 for frappe/payments#53

@frappe frappe deleted a comment from stale bot Sep 5, 2024
@stale stale bot added the inactive label Oct 5, 2024
@blaggacao blaggacao marked this pull request as draft October 5, 2024 23:48
@stale stale bot removed the inactive label Oct 5, 2024
@frappe frappe deleted a comment from stale bot Oct 5, 2024
@stale stale bot added the inactive label Oct 29, 2024
@frappe frappe deleted a comment from stale bot Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants