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

Added ability to reject request actions #184

Merged
merged 2 commits into from
Jul 28, 2020
Merged

Added ability to reject request actions #184

merged 2 commits into from
Jul 28, 2020

Conversation

spoenemann
Copy link
Contributor

It should be possible to reject request actions. This change introduces RejectAction for this purpose.

This is particularly important for requests that are forwarded to a diagram server, e.g. RequestModelAction. We should implement the new feature in sprotty-server, too, after this is merged.

@gitpod-io
Copy link

gitpod-io bot commented Jul 23, 2020

Copy link
Contributor

@JanKoehnlein JanKoehnlein left a comment

Choose a reason for hiding this comment

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

LGTM.

I wonder whether we should by default allow an error JSON object in addition to the message, similar to console.log()

@spoenemann
Copy link
Contributor Author

I thought about that, too, but what type should we use? any?

@JanKoehnlein
Copy link
Contributor

Not any, as these messages are likely to be serialized, but maybe something like in microsoft/TypeScript#1897 (comment)

@spoenemann
Copy link
Contributor Author

I added a detail property to the new action and used the proposed JSON types where I found them appropriate. Please review.

Copy link
Contributor

@JanKoehnlein JanKoehnlein left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

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