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

Start coding style guidelines #4

Open
BillWagner opened this issue Mar 29, 2016 · 6 comments
Open

Start coding style guidelines #4

BillWagner opened this issue Mar 29, 2016 · 6 comments

Comments

@BillWagner
Copy link
Member

This should list the conventions we want to follow.

We should document that we want { } around all if / else clauses.

@mgmccarthy
Copy link

@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...

@BillWagner
Copy link
Member Author

@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
Copy link

@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
Copy link
Member Author

@mgmccarthy Just make a PR for this repo: https://github.com/HTBox/htbox.github.io

Thanks for asking.

@mgmccarthy
Copy link

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
Copy link
Member Author

Pinging @MisterJames for his thoughts.

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

No branches or pull requests

2 participants