MOAB: Mesh Oriented datABase  (version 5.2.1)
styleguide.h
Go to the documentation of this file.
00001 /*!
00002   \page styleguide Coding Style Guide
00003 Code developed in %MOAB should follow the coding styles described here.  Any deviations from this style
00004 guide will result in severe berating and other verbal abuse.
00005
00006 \section dirstructure MOAB Directory Structure
00007 %MOAB source code is organized in the following directory structure: \n
00008  - doc: Documentation is put here, along with the input file for Doxygen.  Most %MOAB documentation is doxygen-processed.
00009  - examples: Examples of %MOAB usage, both small and large.  These programs are not meant to be used as unit tests, but
00010      rather as further documentation on %MOAB usage.
00011  - src : Mesh related source codes. It includes:
00012    - io: %MOAB Input/Output classes.
00013    - moab: %MOAB core classes.
00014    - lotte: Computational Meshing basics.
00015    - parallel: Parallel mesh computation, i/o data processing methods.
00016  - test: All unit test programs should go below this directory.
00017    Please put the unit tests into their related subdirectories based on the test's
00018    purpose if possible.
00019 If you're designing a new class or other code for %MOAB and are not sure where to put it, try to find something similar
00020 and put it there.  Otherwise, email the %MOAB email list for pointers.  <em> In general, you should not need to create new
00021 subdirectories in the %MOAB source code, except when implementing a new algorithm with more than about 2 files.</em>
00022
00023 \section sourcestyle Source Code Style and Best Practices
00024 %MOAB code should abide by the following general rules:
00025  - Names:
00026    - Class names should be in the CamelBack style, e.g. EdgeMesh or VertexMesher.
00027    - Class member variables should be camelBack, e.g. EdgeMesh::schemeType; each member variable, e.g. int memberVariable,
00028    should have set/get functions void member_variable(int newval) and int member_variable(), respectively.
00029    - Enumeration values should be all captitalized, with underscores avoided if possible (the enumeration name indicates
00030      the general purpose of the enumeration, so e.g. we use EQUAL, not EQUAL_MESH)
00031  - Source code should not contain tabs or MS-DOS newlines; tabs and other indentations should be set to a width of 2 spaces.
00032    For general tips on how to set your editor for this, see the %MOAB-dev discussion starting with <a href="https://lists.mcs.anl.gov/mailman/private/moab-dev/2011/000519.html">this message</a>.
00033  - Each class header should be fully commented; that includes:
00034    - A \\file comment block at the top of the file; DO NOT include things like Author and Date blocks; this stuff is available
00035      from subversion if we really need to know.
00036    - A \\class comment block, formatted like those in the %MOAB core classes.  THE FIELDS AFTER THE CLASS NAME ARE VERY IMPORTANT,
00037      as they tell developers how to include the class declaration in their code.  This information goes into the "Detailed
00038      Description" portion of the class documentation.  This block should include any features or limitations of the class.
00039      Eventually, we'll impose some standard boilerplate that each meshing class should use.
00040      Until then, please keep this block to around a paragraph.
00041    - Each function in both the public and private interfaces should be commented, INCLUDING ANY ARGUMENTS AND RETURN VALUES.
00042      See the %MOAB classes for examples of how to format these comments.  As a rule of thumb, your code should run through
00043      Doxygen without generating any warnings; in fact, Doxygen is sometimes helpful at pointing out inconsistencies in your
00044      class declaration.
00045  - Developers should avoid using \#include in header files, as they propagate dependencies more widely than necessary.  The only
00046    cases where other includes are needed are to import the declaration for a parent class, and to declare types used as
00047    non-pointer and non-reference function arguments.  In most cases, a forward-declaration statement (e.g. 'class MKCore')
00048    will suffice.
00049  - Naming classes and other top-level constructs:
00050    - No names should be added to the global namespace.  Everything should be
00051      in the MOAB namespace.  An exception can be made for names with a static
00052      scope declared in a .cpp file, but class member functions never have a
00053      static scope.
00054    - Names should be kept as private as possible.  If declaring a struct or
00055      utility class that is used internally by some other class, consider
00056      defining it in the .cpp file of the main class or a separate header
00057      only included in that .cpp file and using (if necessary) only forward
00058      delcarations (e.g. \c struct \c Point3D;) in the header file used
00059      by other code.  If that is not possible, then consider nesting the
00060      definitions such that the scope of the name is limited to that of the
00061      class using it.
00062    - Any names introduced into the top-level MOAB namespace should be
00063      sufficiently unique to avoid conflicts with other code.  If you must
00064      introduce a class to the top-level MOAB namespace, don't choose
00065      an overly genereric name like \c Point3D .
00066  - Constants and Macros
00067    - Don't use a pre-processor macro where a const variable or an inline or
00068      template function will suffice.
00069      There is absolutely benefit to the former over the later with modern
00070      compilers.  Further, using  macros bypasses typechecking that the compiler
00071      would otherwise do for you and if used in headers, introduce names into
00072      the global rather than MOAB namespace.
00073    - Don't define constants that are already provided by standard libraries.
00074      For example, use \c M_PI as defined in \c math.h rather than defining
00076 \section commits Making Repository Commits
00077 As a general rule, developers should update frequently, and commit changes often.  However, the repository should always remain
00078 in a state where the code can be compiled.  Most of the time, the code should also successfully execute "make check" run from the
00079 top-level directory.  If you commit code that violates this principal, it should be your first priority to return the repository
00080 code to a compilable state, and your second priority to make sure "make check" runs without errors.
00081
00082 Commits to the repository should also come with a non-trivial, useful, non-verbose log message.  Oftentimes the best way to generate
00083 this message is to run 'commit -a', and include a comment on
00084 each file that changed, then Ctrl+O to write out, followed by 'Enter' and Ctrl+X.  Many times it is helpful to state that 'make check runs successfully' at the end of the log message.
00085 Although it would be possible and many software projects do it, we prefer not to force successful execution of the test suite
00086 before every commit.  Developers should make every effort to avoid having to impose this constraint, by running a make check
00087 before every commit.
00088
00089 \section git Git Repository Practices
00090 As most of our code repositories uses git as the revision control system, it is important to decide on a workflow that can be followed by the individual developer. The way that any individual developer interact with the upstream git repository can have an important impact on other developers and the ability to identify and manage individual changes.  This set of guidelines and practices attempts to establish some standards for how developers will interact with the upstream git repository.
00091 The Atlassian website <a href="https://www.atlassian.com/git/workflows">describes a number of git workflows</a> , and provides good background reading for a few standard models of such workflows.  More important than choosing any one of these workflows precisely are the adoption of some of the concepts embodied within them and understanding the implications of those concepts.
00092
00093 \subsection outside-master Working Outside the Master Branch
00094 A critical concept is that all changes shall be developed outside of the master<sup>1</sup> branch.  Whether they are in a different branch of the upstream<sup>2</sup> repository (gitflow) or a branch of an entirely different fork (forking workflow) is secondary.  This is a well-established concept regardless of the workflow being adopted, and allows a number of other benefits as described below.
00095
00096 \subsubsection fork Working on a Different Fork
00097 There are a number of benefits of working on a different fork rather than a branch of the upstream repo, although not strictly technical:
00098 - Developers, particularly new developers, are liberated from the constant oversight of others as they explore new code options.  The impact of this may depend on an individual developer’s personality, but for some it creates a refuge where they can be more free and creative.
00099 - Similarly, assuming that all changesets in the upstream repo are communicated to the entire development team, the team is spared a noisy stream of notifications and can focus their attention on the rarer occurrence of a pull request notification.
00100
00101 \subsubsection pr All Changes are Committed by Pull Request
00102 Although this can be imposed technically by limiting the authority to alter the upstream repo (as in the forking workflow), a healthy developer community can also simply rely on convention.  The advantage of doing it by convention rather than by restriction is that it is easier to distribute the load of reviewing and accepting changes.
00103 A critical consequence of this decision is that all code is reviewed before it is committed to the upstream master branch.  This has benefits to overall quality in two related ways:
00104 - the code under review will improve due to the review itself, and
00105 - those involved in the review will maintain a broad awareness of the code base resulting in better contributions from them.
00106
00107 This practice does, however, place a substantial burden on the developers to perform timely reviews of the pull requested (PR’ed) code.  PR’s that languish without sufficient review have a number of negative consequences:
00108 - they need to be refreshed simply to keep them up-to-date with the possibly advancing upstream/master
00109 - they may delay further development on similar or related features
00110 - they breed frustration in the original developer, undermining the community as a whole.
00111 Bitbucket provides powerful collaboration tools that greatly facilitate this process.
00112
00113 <sup>1</sup> Although a repository may choose a different name for its main development branch, this document will refer to that as the “master” branch.
00114
00115 <sup>2</sup> For this discussion, the “upstream” repo will refer to the centralized authoritative repository used to synchronize changes.
00116
00117 \subsection Some Git Mechanics to Keep it Clean
00118 Given the above practices, there are some mechanical details that can help ensure that the upstream/master repository is always in a state that facilitates all repository actions and interactions.
00119
00120 -# Feature branches being used for development should be kept up-to-date with the upstream/master by rebase only.  When a feature branch is rebased against the upstream/master, all changes in the upstream/master are inserted into the feature branch at a point in its history that is prior to any of the changes of the feature branch.  This can require conflict resultion as the feature branch changes are “replayed” on top of the new upstream/master in its current state.  The primary advantage of this policy is that it keeps all of the feature branch changes contiguous.  If, by contrast, the upstream/master is merged into the feature branch, the recent changes in the upstream/master become woven into the set of changes in the feature branch.  This can make it more difficult to isolate changes later on.
00121
00122     Strict adoption of this practice is important since a single merge into a feature branch that is then merged back into the upstream/master can make it nearly impossible for others to rebase.
00123
00124     A typical workflow with pull-request might look like this, all using the command-line, except for submitting the final pull request.  Note that there is never a merge operation.
00125     -# synchronize your local master branch before anything else
00126     \code
00127      %> git checkout master
00128      %> git fetch upstream
00129      %> git rebase upstream/master
00130      \endcode
00131
00132     -# now create a new feature branch from master
00133     \code
00134      %> git checkout -b my_feature_branch master
00135     \endcode
00136
00137     -# now make changes, editing A.cpp, B.hpp, C.cpp
00138
00140     \code
00141      %> git add A.cpp B.hpp C.cpp
00142      %> git commit -m “Make sure you have a good commit message”
00143     \endcode
00144     -# push your changes to your feature branch on your fork (often called origin)
00145     \code
00146      %> git push origin my_feature_branch
00147     \endcode
00148     -# make more changes, editing B.hpp, D.hpp, E.cpp
00149
00151     \code
00152     %> git add B.hpp D.hpp E.cpp
00153     %> git commit -m “Be sure you have another good commit message”
00154     \endcode
00155     -# push your changes to your freature branch on your fork (often called origin)
00156     \code
00157     %> git push origin my_feature_ranch
00158     \endcode
00159     -# When you are ready to submit a pull request, be sure that your feature branch is up-to-date. This first step may seem redundant but is here to be clear which branch we are acting on
00160     \code
00161     %> git checkout my_feature_branch
00162     %> git fetch upstream
00163     %> git rebase upstream/master
00164     \endcode
00165       This may generate conflicts that can be addressed at this point.
00166
00167       NOTE: This step can be performed at any time and should be performed as often as practical to reduce the scope of potential conflicts.
00168
00169     -# push your updated feature branch on your fork (often called origin)
00170      \code
00171      %> git push origin my_feature_branch
00172      \endcode
00173       This may require the ‘-f’ option to force the push.  (It is frequently necessary to force this push because the act of rebasing will “replay” the commits from the feature branch on top of the master, leading to different commit hashes.  Each of the commits will contain the same actual information, but because it has a different set of hashes, git will think there is an inconsistency and ask you to force the change.)
00174
00175     -# Submit a pull request on Bitbucket, from your fork to the fathomteam fork.
00176
00177
00178 -# When ready to be adopted into the upstream/master, feature branches should be combined by merge only.  This adds the changeset to the end of the upstream/master as a set of individual commits but in a contiguous block.
00179
00180    A typical workflow to merge a pull-request might look like this, all using the command-line.
00181    -# synchronize your local master branch before anything else (just because it’s never a bad idea!)
00182    \code
00183    %> git checkout master
00184    %> git fetch upstream
00185    %> git rebase upstream/master
00186    \endcode
00187    -# add a remote for the user with the pull-request, perhaps the user is ‘other_user’
00188    \code
00189    %> git remote add other_user \
00190          git@bitbucket.org:other_user/moab.git
00191    \endcode
00192    -# fetch the other users repo
00193    \code
00194    %> git fetch other_user
00195    \endcode
00196    -# check out their feature branch
00197    \code
00198    %> git checkout -b pr_feature_branch \
00199        other_user/feature_branch
00200    \endcode
00201    -# confirm that it is up-to-date with the master. This first step may seem redundant but is here to be clear which branch we are acting on
00202    \code
00203    %> git checkout pr_feature_branch
00204    %> git fetch upstream
00205    %> git rebase upstream/master
00206    \endcode
00207    This may generate conflicts that can be addressed at this point.  You may want to request the original author (other_user) take care of these.
00208    -# once confirmed that it’s up-to-date with master, review this branch including:
00210       -building the code
00211       -running tests
00212    -# once satisfied that the code meets the necessary standards and that all required/requested changes are fully incorporated into other_users’s feature branch, merge it into master
00213     \code
00214     %> git checkout master
00215     \endcode
00216     The next two steps may seem redundant but provide some QA
00217     \code
00218     %> git fetch upstream
00219     %> git rebase upstream/master
00220     %> git merge other_user/feature_branch
00221     \endcode
00222    -# push those changes into the master branch on bitbucket
00223     \code
00224     %> git push upstream/master
00225     \endcode
00226
00227 -# When a pull request is open for review, any changes to the feature branch will automatically update the pull request.  This is the appropriate way for a developer to respond to requests for changes that occur through the PR process.
00228
00229
00230 -# If a developer has ongoing work that is based on a feature branch that is under consideration in an open PR, a new feature branch (B) should be created that is based on the previous feature branch (A).  Moreover, as changes are made to the original feature branch (A) due to the review process, the new feature branch (B) should be kept up-to-date by rebase against feature branch (A).  This keeps all subsequent changes of (B) downstream from the changes in (A).  Once feature branch (A) has been adopted into the upstream/master, the new feature branch (B) can start being rebased against the upstream/master instead.
00231
00232
00233 -# When a repo is forked, its branches are not automatically synchronized with the corresponding branches on the upstream repo.  This requires a manual process of synchronization via a local clone.  Assuming that the local repo’s branch has the same name as the upstream branch (<branch>), and that the fork is known as “origin”:
00234     \code
00235      %> git fetch upstream
00236      %> git checkout <branch>
00237      %> git rebase upstream/<branch>
00238      %> git push origin <branch>
00239     \endcode
00240 The decision of which branches to keep up-to-date is up to the developers.  Developers may choose to delete some branches from their own fork to avoid (a) the need to update it and (b) accidentally assuming that it is up-to-date.
00241
00242
00243 -# When rebasing, it is not uncommon to encounter conflicts.  This will interrupt the rebasing process, and each conflicted file will contain conflict blocks.  You will be offered three choices:
00244  - manually resolve the conflicts, add the resolved files with git add,  and then git rebase --continue (do not commit the resolved files!)
00245  - abort the rebasing process entirely with git rebase --abort
00246  - skip the commit that causes the conflict, assuming that you are sure that this is the right thing to do, with git rebase --skip
00247
00248
00249 -# Bitbucket offers a number of buttons/tools to manage changes between branches and forks.  Some of these operate in ways that are contrary to the practices recommended here, and others are consistent with these practices.  In general, it is best to know how to do all of those operations with the command-line instead of relying on Bitbucket, as it gives you full control over important details.
00250
00251
00252 -# During code development, it might be necessary to work on the same branch on different machines. The workflow to update the local branch is to first fetch the remote changes and then perform a hard reset.
00253      \code
00254      %> git fetch origin
00255      %> git reset --hard origin/branch_name
00256      \endcode
00257 One should be careful with the branch name as a hard reset would overwrite all changes in the working directory.
00258
00259
00260 Top: \ref index
00261
00262  */