Merge pull request #251 from twiest/pr

Added 80 character SHOULD to style guide, added a better example for environment in core concepts, added ansible roles flat hierarchy to best practices

3 files changed, 62 insertions, 13 deletions

diff --git a/docs/best_practices_guide.adoc b/docs/best_practices_guide.adoc
index a839cd203..83df53735 100644
--- a/docs/best_practices_guide.adoc
+++ b/docs/best_practices_guide.adoc
@@ -4,7 +4,7 @@
 
 The purpose of this guide is to describe the preferred patterns and best practices used in this repository (both in ansible and python).
 
-It is important to note that this repository may not currently comply with all best practices, but our intention is that it will.
+It is important to note that this repository may not currently comply with all best practices, but the intention is that it will.
 
 All new pull requests created against this repository MUST comply with this guide.
 
@@ -22,14 +22,14 @@ This guide complies with https://www.ietf.org/rfc/rfc2119.txt[RFC2119].
 
 The purpose of this rule is to avoid cases where the build bot will fail pull requests for code modified in a previous pull request.
 
-Our tooling is flexible enough that exceptions can be made so that the tool the build bot is running will ignore certain areas or certain checks, but the build bot itself must pass for the pull request to be merged.
+The tooling is flexible enough that exceptions can be made so that the tool the build bot is running will ignore certain areas or certain checks, but the build bot itself must pass for the pull request to be merged.
 
 
 
 == Python
 
 === PyLint
-http://www.pylint.org/[PyLint] is used in an attempt to keep our python code as clean and as managable as possible. Our build bot runs each pull request through PyLint and any warnings or errors cause the build bot to fail the pull request.
+http://www.pylint.org/[PyLint] is used in an attempt to keep the python code as clean and as managable as possible. The build bot runs each pull request through PyLint and any warnings or errors cause the build bot to fail the pull request.
 
 '''
 [cols="2v,v"]
@@ -49,7 +49,7 @@ Instead, http://docs.pylint.org/faq.html#is-it-possible-to-locally-disable-a-par
 
 .Exceptions:
 1. When PyLint fails because of a dependency that can't be installed on the build bot
-1. When PyLint fails because of including a module that is outside of our control (like Ansible)
+1. When PyLint fails because of including a module that is outside of control (like Ansible)
 
 '''
 [cols="2v,v"]
@@ -78,6 +78,23 @@ metadata[line] = results.pop()
 
 == Ansible
 
+=== Role Directory
+.Context
+* http://docs.ansible.com/playbooks_best_practices.html#directory-layout[Ansible Suggested Directory Layout]
+
+'''
+[cols="2v,v"]
+|===
+| **Rule**
+| The Ansible roles directory MUST maintain a flat structure.
+|===
+
+.The purpose of this rule is to:
+* Comply with the upstream best practices
+* Make it familiar for new contributors
+* Make it compatible with Ansible Galaxy
+
+
 === Filters
 .Context:
 * https://docs.ansible.com/playbooks_filters.html[Ansible Playbook Filters]
diff --git a/docs/core_concepts_guide.adoc b/docs/core_concepts_guide.adoc
index fa2421eec..38187c55e 100644
--- a/docs/core_concepts_guide.adoc
+++ b/docs/core_concepts_guide.adoc
@@ -4,7 +4,7 @@
 
 The purpose of this guide is to describe core concepts used in this repository.
 
-It is important to note that this repository may not currently implement all of the concepts, but our intention is that it will.
+It is important to note that this repository may not currently implement all of the concepts, but the intention is that it will.
 
 == Logical Grouping Concepts
 The following are the concepts used to logically group OpenShift cluster instances.
@@ -35,7 +35,8 @@ Defined Cluster Group:
 === Environment
 An environment is a logical grouping of one or more cluster groups. How the environment is defined is determined by the OpenShift administrators.
 
-Example: Extending the two examples above, Hosting Corp could provide both production and staging environments for it's customers. In this way, Acme Corp could push their code to the staging environment and test it out before pushing it to production.
+Example: Extending the two examples above, Hosting Corp is upgrading to the latest version of OpenShift. Before deploying it to their clusters in the Production environment, they want to test it out. So, Hosting Corp runs an Ansible playbook specifically against all of the cluster groups in the Staging environment in order to do the OpenShift upgrade.
+
 
 Defined Environments:
 `production`
diff --git a/docs/style_guide.adoc b/docs/style_guide.adoc
index b4c99e1d9..3b888db12 100644
--- a/docs/style_guide.adoc
+++ b/docs/style_guide.adoc
@@ -4,15 +4,46 @@
 
 The purpose of this guide is to describe the preferred coding conventions used in this repository (both in ansible and python).
 
-It is important to note that this repository may not currently comply with all style guide rules, but our intention is that it will.
+It is important to note that this repository may not currently comply with all style guide rules, but the intention is that it will.
 
 All new pull requests created against this repository MUST comply with this guide.
 
 This style guide complies with https://www.ietf.org/rfc/rfc2119.txt[RFC2119].
 
-== Ansible Variable Naming
+== Python
 
-=== Global Variables
+
+=== Python Maximum Line Length
+
+.Context:
+* https://www.python.org/dev/peps/pep-0008/#maximum-line-length[Python Pep8 Line Length]
+
+'''
+[cols="2v,v"]
+|===
+| **Rule**
+| All lines SHOULD be no longer than 80 characters.
+|===
+
+Every attempt SHOULD be made to comply with this soft line length limit, and only when it makes the code more readable should this be violated.
+
+Code readability is subjective, therefore pull-requests SHOULD still be merged, even if they violate this soft limit as it is up to the individual contributor to determine if they should violate the 80 character soft limit.
+
+
+'''
+[cols="2v,v"]
+|===
+| **Rule**
+| All lines MUST be no longer than 120 characters.
+|===
+
+This is a hard limit and is enforced by the build bot. This check MUST NOT be disabled.
+
+
+
+== Ansible
+
+=== Ansible Global Variables
 Ansible global variables are defined as any variables outside of ansible roles. Examples include playbook variables, variables passed in on the cli, etc.
 
 '''
@@ -29,7 +60,7 @@ Example:
 g_environment: someval
 ----
 
-==== Role Variables
+=== Ansible Role Variables
 Ansible role variables are defined as variables contained in (or passed into) a role.
 
 '''
@@ -39,7 +70,7 @@ Ansible role variables are defined as variables contained in (or passed into) a
 | Role variables MUST have a prefix of atleast 3 characters. See below for specific naming rules.
 |===
 
-===== Role with 3 (or more) words in the name
+==== Role with 3 (or more) words in the name
 
 Take the first letter of each of the words.
 
@@ -61,7 +92,7 @@ tmur_var1: value_one
 
 
 
-===== Role with 2 (or less) words in the name
+==== Role with 2 (or less) words in the name
 
 Make up a prefix that makes sense.
 
@@ -82,7 +113,7 @@ tow_var1: value_one
 ----
 
 
-===== Role name prefix conflicts
+==== Role name prefix conflicts
 If two role names contain words that start with the same letters, it will seem like their prefixes would conflict.
 
 Role variables are confined to the roles themselves, so this is actually only a problem if one of the roles depends on the other role (or uses includes into the other role).