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: Add push consent & display fees #21

Merged
merged 8 commits into from
Feb 13, 2024
Merged

Conversation

pawanpaudel93
Copy link
Contributor

@pawanpaudel93 pawanpaudel93 commented Feb 8, 2024

Summary

This PR adds improvements by showing a push consent before pushing with fees. It also adds the ability to set the threshold cost above which the consent (y/n) is shown for push.

How to test

  1. Install dependencies, build and install remote helper
pnpm install && pnpm build && npm install -g .
  1. Run PL locally with contract id as xw8kD7s33ElIEioN96sPmLTJj9Rs2v4dZ67KWWKqojQ and create a repo or use already created repo there.
  2. Clone the repo, make some changes and push the changes. See info about on how to set threshold cost here

@kranthicodes
Copy link
Member

Great job with the flow. Works as expected but a small suggestion or change request. Can we handle a scenario where user chooses 'n' to not push and sees relavant error message like, Pushing to origin was cancelled.

Screenshot 2024-02-09 at 1 27 08 AM

@pawanpaudel93
Copy link
Contributor Author

Thanks for the suggestion @kranthicodes. Yes i do agree we should show a different message when user cancels the push and i will make changes to do that.

Copy link
Member

@kranthicodes kranthicodes left a comment

Choose a reason for hiding this comment

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

tACK

Copy link
Collaborator

@7i7o 7i7o left a comment

Choose a reason for hiding this comment

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

tACK
LGTM! 🚀

@kranthicodes kranthicodes merged commit 35bf96f into main Feb 13, 2024
1 check passed
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.

3 participants