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: added missing events in the resource rental module #1005

Merged
merged 3 commits into from
Jun 28, 2024

Conversation

mgordel
Copy link
Contributor

@mgordel mgordel commented Jun 28, 2024

No description provided.

@mgordel mgordel requested review from SewerynKras and grisha87 June 28, 2024 11:33
exeUnitDestroyed: (activity: Activity) => void;

/** Emitted when there is an error while destroying the ExeUnit */
errorDestroyingExeUnit: (error: Error) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would just keep this as error and then leave the details of what happened to the error itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, in this case it may look better, I suggested other modules where we have it distinguished in this way and I wanted to keep consistency

exeUnitCreated: (activity: Activity) => void;

/** Emitted when there is an error while creating the ExeUnit */
errorCreatingExeUnit: (error: Error) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would just keep this as error and then leave the details of what happened to the error itself.

@mgordel mgordel requested a review from grisha87 June 28, 2024 12:11
@grisha87 grisha87 merged commit 8f104cb into beta Jun 28, 2024
8 checks passed
@mgordel mgordel deleted the mgordel/JST-933/rental-events branch September 6, 2024 11:19
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.

2 participants