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

Document best practices, conventions and common usage problems. #135

Open
jmalloc opened this issue Nov 5, 2020 · 16 comments
Open

Document best practices, conventions and common usage problems. #135

jmalloc opened this issue Nov 5, 2020 · 16 comments
Labels
documentation Change or addition to documentation

Comments

@jmalloc
Copy link
Member

jmalloc commented Nov 5, 2020

We're starting to get quite a collection of Dogma conventions that we've discussed in person, on slack or on PRs. We should document those for others to see.

Best Practices

Here's some things to get us started that I think are definitely becoming our "best practices":

  • naming conventions for message types (imperative for commands, past tense passive for events, etc)
  • grammatical mood / tense for message descriptions (and whether there is an "actor" mentioned)
  • app/handler keys should be UUIDs this is now a hard requirement
  • implement business logic outside the handler itself, in the domain objects as is standard with DDD Document best practices, conventions and common usage problems. #135 (comment)
  • data retention policies for projections

Style Guide / Conventions

And some additional things that we've adopted as conventions, but I'm not sure yet whether they're really a "best practice" or just a convention we've adopted somewhat arbitrarily. Either way we might want to make them part of a style guide.
ed)

Usage Problems / Gotchas / FAQ

Edit Adding a third section for things that aren't really optional, but still catch you out. Possible for a FAQ / gotchyas type document:


I'm sure there are more, LMK if you can think of any.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 5, 2020

@danilvpetrov @koden-km I could really use some help with this one. Even if it's just remembering things we have discussed/decided, or making sure to update this issue if we settle on any of the best practices in the future.

I'm sure there's a lot to extract from those recent cwx PRs for those new domains.

@danilvpetrov
Copy link
Member

are we including test conventions too, even framework-specific (i.e. Ginkgo)? If so, the one i was stung by was that When() blocks should describe the state that the app is in, not the action taken.

@danilvpetrov
Copy link
Member

danilvpetrov commented Nov 5, 2020

Another one i can think of (which is IMHO quite critical) is to change aggregate root state ONLY in .ApplyEvent() when reacting to the event produced.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 5, 2020

are we including test conventions too, even framework-specific (i.e. Ginkgo)?

I would be happy to include testkit conventions, but probably not Ginkgo related in this document. The example project doesn't use Ginkgo either, for what it's worth.

We could definitely do that in a cwx guide, but IMO that is really part of just using Ginkgo/BDD style properly.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 5, 2020

Another one i can think of (which is IMHO quite critical) is to change aggregate root state ONLY in .HandleEvent() when reacting to the event produced.

This one isn't a "best practice", it's absolutely mandatory, and is documented as such in the API documentation. That said, maybe we could have a FAQ / common problems document too.

@jmalloc jmalloc changed the title Create a "best practices" document. Document best practices, conventions and common usage problems. Nov 5, 2020
@danilvpetrov
Copy link
Member

Ok, one of best practice item could be to allocate basic idempotency checks and producing an event into seperate methods in aggregate roots. That would make .HandleCommand() less busy and readable.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 5, 2020

I've updated the issue title and initial comment to include a "FAQ" section as well.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 5, 2020

It might also be worth familiarising yourselves with the issues I've created under https://github.com/dogmatiq/dogmavet/issues. There's probably a lot of crossover. We could definitely have dogmavet warn when you're not using best practices.

There's potentially some issues there that are out of date with the recent spec changes too.

@danilvpetrov
Copy link
Member

May be it's worth even mentioning, that .HandleCommand() should generally consist of two sections:

  1. Root type assertion (I think it's still valid even if Pass AggregateRoot directly to HandleCommand() #133 is addressed)
  2. switch block that calls aggregate root methods for each command handled with compulsary panicking within default branch.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 5, 2020

May be it's worth even mentioning, that .HandleCommand() should generally consist of two sections [...]

This might belong in a "style guide" section, perhaps?

@danilvpetrov
Copy link
Member

danilvpetrov commented Nov 5, 2020

May be it's worth mentioning space-separation of the field declarations in .proto files as per @koden-km earlier suggestions.

This might belong to the style guide as well.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 5, 2020

May be it's worth mentioning space-separation of the field declarations in .proto files as per @koden-km earlier suggestions.

I think this one is getting too prescriptive and specific, especially considering that dogmatiq/dogma itself is unaware of protocol buffers.

@jmalloc jmalloc added the documentation Change or addition to documentation label Nov 5, 2020
@danilvpetrov
Copy link
Member

danilvpetrov commented Nov 5, 2020

Regardless of test framework, we should separate our test files by the domain action being tested and name them using the action, e.g.: assign_test.go, requestcancel_test.go.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 5, 2020

Regardless of test framework, we should separate our test files

Isn't this just normal Go?

@danilvpetrov
Copy link
Member

danilvpetrov commented Nov 5, 2020

Hmm, IMHO normal Go would call the test file the same as the file being tested, e.g: foodomain.go -> foodomain_test.go.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 5, 2020

TBH, I feel like that's a convention we carried over from PHP more than anything. I suppose we still do that often, but I have plenty of packages with tests organised differently to that.

Either way, I don't think it's outside the scope of what Dogma documentation should be recommending.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Change or addition to documentation
Projects
Development

No branches or pull requests

2 participants