Before making changes to the DotNetOpenAuth source code, be sure you've followed the quick start steps.
Code ownership
Notwithstanding this is an open-source project, there is a sense of ownership in the code contributors submit via check-in or patches. Ownership can explicitly transfer when the owner asks or authorizes someone else to take over a feature. Ownership can implicitly transfer if a contributor has not participated by email or coding contributions in the last several months and someone else picks up ownership, usually by studying the code and fixing bugs in it.
Coding etiquette suggests that before you change code you didn't write, you should email the owner of the code and clear it with him or her first, perhaps with the proposed patch.
Coding Style
- Tabs, not spaces. And curly-braces usually not on their own lines. See quick start for how to configure VS' auto-formatting to fit these rules.
- Comply with all StyleCop rules, including the ones added in quick start.
Committing changes
Generally speaking, commits are bug fixes or feature work, and there should be tickets open for the work you're doing and assigned to you before you begin work so others know what you're working on and don't do work in the same area.
All commits should be carefully scoped. Resist the urge to change anything not directly related to the task you're working on. If you see another change you'd like to make, make a separate commit for that change. Some commits might be "Fixed a bunch of FxCop messages" and that's fine. But it's not fine to fix some FxCop messages in foo.cs while implementing a feature in bar.cs and commit them both together.
Before git commit, review these:
- ALWAYS review the diff of your change before committing it and make sure that there is no diff noise from frivolous whitespace changes or irrelevant code churn.
- DO write unit tests to verify a fix or feature and include in the same or the next commit.
- DO make many commits as necessary to isolate changes. For instance, if you are adding a feature and find a bug in the process, fix the bug and check that fix in separately, then resume work on your feature.
- DO break a large feature into several smaller commits if it is architecturally sound to do so and each commit adds value.
- DON'T fix multiple bugs in one commit if you can avoid it.
- DON'T break the build. Every commit should build and (preferably) pass unit tests.
- DO verify that any .js changes you made in the library minify in release builds without introducing errors (do not omit required semicolons).
Before git push, make sure you satisfy all these criteria:
- StyleCop should run clean (Ctrl+Shift+Y in the IDE).
- All unit tests should pass.
- FxCop should run clean.
Remember that once you push, you must not rewrite history. Any corrections you need to make must be an additional commit.
After a git push, be sure to resolve any tickets your work has completed.
Goals
As many developers from various OS and IDE backgrounds contribute to this project, we seek to achieve a uniform coding style for these reasons:
- Stay productive by focusing on code content and not perpetually debating or changing coding style.
- Easy reading and maybe fewer bugs if we're lucky.
- Predictable conventions based on generally accepted industry practice for C# programs and libraries.
- Leverage the power of .NET wherever possible to reduce code size and testing requirements.
- Allows for clean FxCop (code analysis) runs with an agreed upon set of rules.
- Minimize whitespace code churn. Diffs between versions should cleanly represent changes to code and not spurious changes due only to whitespace formatting changes made by an IDE or an errant programmer.
Strict rules
- Exception messages should never include confidential information (association secrets, usernames, etc.) as these can and do get sent to untrusted remote parties in some cases.
