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

Fix typo in documentation regarding changing inbox to outbox #1741

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

dimabarbul
Copy link
Contributor

@dimabarbul dimabarbul commented Sep 10, 2023

The text below suggests that path in response has changed from /inbox/... to /outbox/... But the example JSON does not shows this change.

@thjaeckle
Copy link
Member

In fact, this change from inbox to outbox is not required, it is more of a relict of wrong documentation - I think the example once had this change to outbox also contained.

I am wondering if it would make more sense to delete this hint about changing "inbox" to "outbox". It does not really make things clearer - instead more confusing.

  • a message response is already identified by the use of status in the payload
  • it is correlated via the correlation-id header
  • no permissions are checked - knowing the correlation-id is sufficient to answer to a message

Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

I would rather remove the sentence:

Notice, that the `path` of the Message has changed from *inbox* to *outbox*, 
which means the Message was sent *from* the Thing.

@thjaeckle thjaeckle added this to the 3.3.7 milestone Sep 11, 2023
@thjaeckle
Copy link
Member

LGTM, thanks 👍

@thjaeckle thjaeckle merged commit 115d3fa into eclipse-ditto:master Sep 11, 2023
1 check passed
@dimabarbul dimabarbul deleted the inbox-outbox-typo branch September 11, 2023 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants