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

Move VoucherExecution component under packages/ui #129

Closed
4 tasks
nevendyulgerov opened this issue Mar 7, 2024 · 3 comments
Closed
4 tasks

Move VoucherExecution component under packages/ui #129

nevendyulgerov opened this issue Mar 7, 2024 · 3 comments
Assignees
Labels
Status: Discussion Collaboratively refine some idea / feature Type: Refactor Code changes that neither adds a new feature nor fix a bug.

Comments

@nevendyulgerov
Copy link
Contributor

📄 Context

As per this comment move the VoucherExecution component under packages/ui.

📈 Subtasks

  • VoucherExecution is moved to packages/ui
  • Test suite for VoucherExecution is moved to packages/ui

🎯 Definition of Done

  • Executing a voucher works ok
  • Tests for voucher execution are passing
@nevendyulgerov nevendyulgerov added the Type: Refactor Code changes that neither adds a new feature nor fix a bug. label Mar 7, 2024
@nevendyulgerov nevendyulgerov self-assigned this Mar 7, 2024
@github-project-automation github-project-automation bot moved this to 📌 Todo in Explorer Unit Mar 7, 2024
@brunomenezes brunomenezes added the Status: Discussion Collaboratively refine some idea / feature label Mar 7, 2024
@brunomenezes
Copy link
Collaborator

Hi @nevendyulgerov, my comment was to discuss the motivation behind the current choice rather than move it directly.
For example, a few questions on that end;

  • What was the motivation to add to the apps/web instead of the packages/ui?
  • Is that because of the types generated by our codegen tools? (Generators are good if you accept that sometimes some work will be needed on rough edges).
  • Is that related to extensibility?

@nevendyulgerov
Copy link
Contributor Author

nevendyulgerov commented Mar 8, 2024

@brunomenezes , the motivation was the types coming from the codegen located in apps/web.

In VoucherExecution we are using a prop voucher with interface Voucher coming from graphql/rollups/types. Initially, my plan was to place this component under packages/ui, however due to this type already existing in apps/web I decided to keep it there to prevent interface duplication.

@nevendyulgerov
Copy link
Contributor Author

I think we can close this one for the time being.

@github-project-automation github-project-automation bot moved this from 📌 Todo to 📦 Done in Explorer Unit Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Discussion Collaboratively refine some idea / feature Type: Refactor Code changes that neither adds a new feature nor fix a bug.
Projects
Archived in project
Development

No branches or pull requests

2 participants