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

Add API documentation for 'MessageService' #8688

Conversation

sdirix
Copy link
Contributor

@sdirix sdirix commented Oct 29, 2020

Closes #8687

Signed-off-by: Stefan Dirix sdirix@eclipsesource.com
Contributed on behalf of STMicroelectronics

What it does

Document MessageService and related message interfaces.

How to test

Check API doc content and whether the linter reports any formatting errors.

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added documentation issues related to documentation messaging issues related to messaging labels Oct 29, 2020
@sdirix sdirix force-pushed the messageservice-documentation branch from 63d26a5 to 7b3c9fc Compare October 30, 2020 07:23
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look very good, I have two minor comments.

packages/core/src/common/message-service.ts Outdated Show resolved Hide resolved
Comment on lines 31 to 33
* The messages are processed by this service and forwarded to an injected {@link MessageClient}.
* Usually this is the {@link NotificationManager} contributed by "@theia/messages" rendering
* messages as notifications in the frontend.
Copy link
Member

Choose a reason for hiding this comment

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

Do you believe we should reference @theia/messages (a downstream extension) in the parent @theia/core?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just wanted to give an example of a MessageClient. I assume that many apps will use the NotificationManager so when someone looks at what the MessageService does they know that their messages end up there. So personally I think it's fine to list it as an example here, but if you prefer I could also remove it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the link and reworded it a bit. If you prefer to remove the mention of "@theia/messages" completely, just let me know :)

Document 'MessageService' and related message interfaces.

Signed-off-by: Stefan Dirix <sdirix@eclipsesource.com>
Contributed on behalf of STMicroelectronics
@sdirix sdirix force-pushed the messageservice-documentation branch from 7b3c9fc to c945385 Compare November 5, 2020 16:49
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look good to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation issues related to documentation messaging issues related to messaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add API documentation for 'MessageService'
2 participants