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(guidelines): add new rule for sending durable event ids #77

Merged
merged 2 commits into from
Oct 7, 2024

Conversation

JanKlasser3000
Copy link
Contributor

@JanKlasser3000 JanKlasser3000 commented Aug 9, 2024

Changelog:

New

  • SHOULD send durable event IDs R000058

Copy link

@ccudennec-otto ccudennec-otto left a comment

Choose a reason for hiding this comment

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

@maxedenharter0507
Copy link
Contributor

I feel this is strongly related to idempotency. What do you think about linking the article? https://github.com/otto-de/api-guidelines/blob/15c925dedcb3d474b17d33fc35fc6015f5289c2c/dev-context/async/03-decisions/consumption-idempotency.md

The dev-context section is not linkable in our API portal. But feel free to link the corresponding guideline: https://api.otto.de/portal/guidelines/r200002

@maxedenharter0507
Copy link
Contributor

Like that rule, maybe we could also add a link to https://api.otto.de/portal/guidelines/event-guidelines/concepts#events as we already quote it.

@BirgitBader
Copy link
Contributor

Hi @JanKlasser3000 , thanks for this PR 🚀
From a technical writing perspective, I suggest the following:

For the Changelog (PR body):

New

  • SHOULD send durable event IDs R000058

For the PR title (I know, we don't have a convention for that yet, but ...😉)

feat(guidelines): add new rule for sending durable event ids

Copy link
Contributor

@BirgitBader BirgitBader left a comment

Choose a reason for hiding this comment

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

@JanKlasser3000 I've made some remarks.

@JanKlasser3000
Copy link
Contributor Author

Thanks for all the input and I will do the changes about the wording.

I agree about the correlation for the differenct sections, however I think it can also be confusing to add to many links and makes it a bit hard to get an overview. Could we also move the concept in the menu up, so its clear its valid for all the rules? For the idempotency a link can be useful, maybe the other way round, to indicate that event IDs can be used for idempotency checking?

As for the title, its the best I could come up with, however I am open for suggestions

@maxedenharter0507
Copy link
Contributor

Thanks for all the input and I will do the changes about the wording.

I agree about the correlation for the differenct sections, however I think it can also be confusing to add to many links and makes it a bit hard to get an overview. Could we also move the concept in the menu up, so its clear its valid for all the rules? For the idempotency a link can be useful, maybe the other way round, to indicate that event IDs can be used for idempotency checking?

As for the title, its the best I could come up with, however I am open for suggestions

Linking the other way around makes sense to me. For me, it's also okay to leave the link to the events section to not overwhelm the reader.

@JanKlasser3000 JanKlasser3000 changed the title feat: add new rule for durable event IDs feat(guidelines): add new rule for sending durable event ids Oct 4, 2024
@JanKlasser3000
Copy link
Contributor Author

I changed the PR, ready for approval ( in case nothing new is requested )

@BirgitBader BirgitBader self-requested a review October 7, 2024 05:49
@BirgitBader BirgitBader merged commit e545401 into otto-de:main Oct 7, 2024
1 check passed
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.

4 participants