You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@BillWagner, would also be nice to have some guidance here on naming conventions as well, in particular, around how Queries, Commands, Notifications and Handlers should be named when using MediatR.
Do you want me to take an initial crack at those conventions? They would be similar to how commands and events are named for NServiceBus. aka...
a command is in the present tense and is an order: MakeCustomerPreferred
an event (in MediatR's case, a Notfication) is in the past tense and says something has been done: CustomerMadePrefferred
I'd also like to drop the convention of naming class files for message with the name of the type of message they represent. aka, ShowActivityCommand becomes ShowActivity
for Queries, we might want to think about prepending Get on the name of the messages?
for Notifcations, since they're a message multiple subscribers are interested in, they represent an "event"... so something like VolunteerSignupNotification becomes VolunteerSignedUp. A good example of existing naming for this in the project is the UserUnenrolls notification... but it should probably be called UserUnenrolled
And one more thing to consider. Do we want the word Async on the handlers that are async? Same goes for the messages? My two cents, keep the word Async on the handlers, but drop them from the messages.
These changes might spur some renaming of existing MediatR messages and their handlers to something the coincide closer to the domain language of allReady and could lend some clarity to new contributors.
In fact, where the word Async is used on the naming of a file could also go into conventions.
The same thing for unit tests. Should all unit tests have the word Tests appended to the name of the class they're testing? That's what I've been doing so far, yet I see some people naming their tests like [ThisClassName]Should...
@MisterJames actually recommended to me the other day to name my tests class names with "Should" at the end of the name...
We can also include what HttpStatusCode we should returning from our controllers that return HttpStatus codes (aka, "Web API" controllers, even though there is no such thing anymore in Core 1).
Activity
mgmccarthy commentedon Mar 29, 2016
@BillWagner, would also be nice to have some guidance here on naming conventions as well, in particular, around how Queries, Commands, Notifications and Handlers should be named when using MediatR.
Do you want me to take an initial crack at those conventions? They would be similar to how commands and events are named for NServiceBus. aka...
a command is in the present tense and is an order:
MakeCustomerPreferredan event (in MediatR's case, a Notfication) is in the past tense and says something has been done:
CustomerMadePrefferredI'd also like to drop the convention of naming class files for message with the name of the type of message they represent. aka,
ShowActivityCommandbecomesShowActivityfor Queries, we might want to think about prepending
Geton the name of the messages?for Notifcations, since they're a message multiple subscribers are interested in, they represent an "event"... so something like
VolunteerSignupNotificationbecomesVolunteerSignedUp. A good example of existing naming for this in the project is theUserUnenrollsnotification... but it should probably be calledUserUnenrolledAnd one more thing to consider. Do we want the word
Asyncon the handlers that are async? Same goes for the messages? My two cents, keep the wordAsyncon the handlers, but drop them from the messages.These changes might spur some renaming of existing MediatR messages and their handlers to something the coincide closer to the domain language of allReady and could lend some clarity to new contributors.
In fact, where the word Async is used on the naming of a file could also go into conventions.
The same thing for unit tests. Should all unit tests have the word
Testsappended to the name of the class they're testing? That's what I've been doing so far, yet I see some people naming their tests like [ThisClassName]Should...@MisterJames actually recommended to me the other day to name my tests class names with "Should" at the end of the name...
BillWagner commentedon Mar 29, 2016
@mgmccarthy Great feedback. Yes, if you want to start a draft, based on what you've seen in the code so far, that would be fantastic.
Also, pinging @MisterJames for his input on this item.
mgmccarthy commentedon Mar 29, 2016
@BillWagner where do I work on/store this draft? Can it just be a local word doc or is there somewhere I can work on this via GitHub?
BillWagner commentedon Mar 29, 2016
@mgmccarthy Just make a PR for this repo: https://github.com/HTBox/htbox.github.io
Thanks for asking.
mgmccarthy commentedon Mar 30, 2016
We can also include what HttpStatusCode we should returning from our controllers that return HttpStatus codes (aka, "Web API" controllers, even though there is no such thing anymore in Core 1).
Most of the recommendations are from people asking on StackOverflow:
http://stackoverflow.com/questions/2342579/http-status-code-for-update-and-delete
http://stackoverflow.com/questions/1860645/create-request-with-post-which-response-codes-200-or-201-and-content
Not too sure what allReady's approach is to returning status codes, and I don't want to make any assumptions without guidance.
BillWagner commentedon Mar 31, 2016
Pinging @MisterJames for his thoughts.