saltstack/salt
1
0
Fork 0
mirror of https://github.com/saltstack/salt.git synced 2025-04-17 10:10:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Update contributing docs

Browse source 
We've changed to a `master` branching strategy, and we'll be working
with `black` and `pre-commit`. So there's that.
This commit is contained in:
Wayne Werner 2019-12-30 14:31:59 -06:00
parent e79462cb4c
commit b7baa79944
No known key found for this signature in database
GPG key ID: C36D3A8D5BEF0935
2 changed files with 148 additions and 465 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

416
doc/topics/development/contributing.rst
View file

@ -4,39 +4,74 @@
Contributing
============
There is a great need for contributions to Salt and patches are welcome! The goal
here is to make contributions clear, make sure there is a trail for where the code
has come from, and most importantly, to give credit where credit is due!
There is a great need for contributions to Salt and patches are welcome! The
goal here is to make contributions clear, make sure there is a trail for where
the code has come from, and most importantly, to give credit where credit is
due!
There are a number of ways to contribute to Salt development.
There are a number of ways to contribute to Salt development, including (but
not limited to):
For details on how to contribute documentation improvements please review
:ref:`Writing Salt Documentation <salt-docs>`.
* filing well-written bug reports
* enhancing the documentation
* providing workarounds, patches, and other code without tests
* engaging in constructive discussion
* helping out in `#salt on Freenode <#salt on freenode_>`_,
the `Community Slack <SaltStack Community Slack_>`_,
the `salt-users <salt-users_>`_ mailing list,
a `SaltStack meetup <saltstack meetup_>`_,
or `Server Fault <saltstack on serverfault_>`_.
* telling others about problems you solved with Salt
If this or other Salt documentation is unclear, please review :ref:`Writing
Salt Documentation <salt-docs>`. PRs are welcome!
Quickstart
----------
If you just want to get started before reading the rest of this guide, you can
get the process started by running the following:
.. code-block:: bash
python3 -m pip install --user pre-commit
git clone --origin upstream https://github.com/saltstack/salt.git
cd salt
pre-commit install
While those commands are running, finish reading the rest of this guide.
Pre-commit
----------
To reduce friction during the development process, SaltStack uses `pre-commit
<pre-commit_>`_. This tool adds pre-commit hooks to git to automate several
processes that used to be manual. Rather than having to remember to run several
different tools before you commit, you only have to run ``git commit``, and you
will be notified about style and lint issues before you ever open a PR.
Salt Coding Style
-----------------
SaltStack has its own coding style guide that informs contributors on various coding
approaches. Please review the :ref:`Salt Coding Style <coding-style>` documentation
for information about Salt's particular coding patterns.
SaltStack is `joining the ranks <SEP 15_>`_ of projects in adopting the `Black
code formatter <Black_>`_ in order to ease the adoption of a unified code
formatting style.
Where Black is silent, SaltStack has its own coding style guide that informs
contributors on various style points. Please review the :ref:`Salt Coding Style
<coding-style>` documentation for information about Salt's particular coding
patterns.
Within the :ref:`Salt Coding Style <coding-style>` documentation, there is a
section about running Salt's ``.testing.pylintrc`` file. SaltStack recommends
running the ``.testing.pylintrc`` file on any files you are changing with your
code contribution before submitting a pull request to Salt's repository. Please
see the :ref:`Linting<pylint-instructions>` documentation for more information.
.. note::
There are two pylint files in the ``salt`` directory. One is the
``.pylintrc`` file and the other is the ``.testing.pylintrc`` file. The
tests that run in Jenkins against GitHub Pull Requests use
``.testing.pylintrc``. The ``testing.pylintrc`` file is a little less
strict than the ``.pylintrc`` and is used to make it easier for contributors
to submit changes. The ``.pylintrc`` file can be used for linting, but the
``testing.pylintrc`` is the source of truth when submitting pull requests.
code contribution before submitting a pull request to Salt's repository.
If you've installed ``pre-commit``, this will automatically happen before each
commit. Otherwise, see the :ref:`Linting<pylint-instructions>` documentation
for more information.
.. _github-pull-request:
@ -48,7 +83,8 @@ contributions. The workflow advice below mirrors `GitHub's own guide <GitHub
Fork a Repo Guide_>`_ and is well worth reading.
#. `Fork saltstack/salt`_ on GitHub.
#. Make a local clone of your fork.
#. Make a local clone of your fork. (Skip this step if you followed
the Quickstart)
.. code-block:: bash
@ -61,6 +97,12 @@ Fork a Repo Guide_>`_ and is well worth reading.
git remote add upstream https://github.com/saltstack/salt.git
If you followed the Quickstart, you'll add your own remote instead
.. code-block:: bash
git remote add my-account git@github.com:my-account/salt.git
#. Create a new branch in your clone.
.. note::
@ -69,47 +111,34 @@ Fork a Repo Guide_>`_ and is well worth reading.
feature Y". Multiple unrelated fixes and/or features should be
isolated into separate branches.
If you're working on a bug or documentation fix, create your branch from
the oldest **supported** main release branch that contains the bug or requires the documentation
update. See :ref:`Which Salt Branch? <which-salt-branch>`.
.. code-block:: bash
git fetch upstream
git checkout -b fix-broken-thing upstream/2016.11
If you're working on a feature, create your branch from the |repo_primary_branch| branch.
.. code-block:: bash
git fetch upstream
git checkout -b add-cool-feature upstream/|repo_primary_branch|
git checkout -b fix-broken-thing upstream/master
#. Edit and commit changes to your branch.
.. code-block:: bash
vim path/to/file1 path/to/file2
vim path/to/file1 path/to/file2 tests/test_file1.py tests/test_file2.py
git diff
git add path/to/file1 path/to/file2
git commit
Write a short, descriptive commit title and a longer commit message if
necessary.
necessary. Use an imperative style for the title.
.. note::
GOOD
If your change fixes a bug or implements a feature already filed in the
`issue tracker`_, be sure to
`reference the issue <https://help.github.com/en/articles/closing-issues-using-keywords>`_
number in the commit message body.
.. code-block:: bash
.. code-block::
Fix broken things in file1 and file2
Fixes #31337
We needed to make this change because the underlying dependency
changed. Now this uses the up-to-date API.
# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
# On branch fix-broken-thing
@ -117,6 +146,30 @@ Fork a Repo Guide_>`_ and is well worth reading.
# modified: path/to/file1
# modified: path/to/file2
BAD
.. code-block::
Fixes broken things
# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
# On branch fix-broken-thing
# Changes to be committed:
# modified: path/to/file1
# modified: path/to/file2
Taking a few moments to explain *why* you made a change will save time
and effort in the future when others come to investigate a change. A
clear explanation of why something changed can help future developers
avoid introducing bugs, or breaking an edge case.
.. note::
If your change fixes a bug or implements a feature already filed in the
`issue tracker`_, be sure to
`reference the issue <https://help.github.com/en/articles/closing-issues-using-keywords>`_
number in the commit message body.
If you get stuck, there are many introductory Git resources on
http://help.github.com.
@ -141,17 +194,9 @@ Fork a Repo Guide_>`_ and is well worth reading.
.. code-block:: bash
git fetch upstream
git rebase upstream/2016.11 fix-broken-thing
git rebase upstream/master fix-broken-thing
git push -u origin fix-broken-thing
or
.. code-block:: bash
git fetch upstream
git rebase upstream/|repo_primary_branch| add-cool-feature
git push -u origin add-cool-feature
If you do rebase, and the push is rejected with a
``(non-fast-forward)`` comment, then run ``git status``. You will
likely see a message about the branches diverging:
@ -180,18 +225,11 @@ Fork a Repo Guide_>`_ and is well worth reading.
https://github.com/my-account/salt/pull/new/fix-broken-thing
#. If your branch is a fix for a release branch, choose that as the base
branch (e.g. ``2016.11``),
https://github.com/my-account/salt/compare/saltstack:2016.11...fix-broken-thing
If your branch is a feature, choose ``|repo_primary_branch|`` as the base branch,
https://github.com/my-account/salt/compare/saltstack:|repo_primary_branch|...add-cool-feature
#. Choose ``master`` as the base Salt branch.
#. Review that the proposed changes are what you expect.
#. Write a descriptive comment. Include links to related issues (e.g.
'Fixes #31337.') in the comment field.
#. Write a descriptive comment. If you added good information to your git
commit message, they will already be present here. Include links to
related issues (e.g. 'Fixes #31337.') in the comment field.
#. Click ``Create pull request``.
#. Salt project members will review your pull request and automated tests will
@ -209,8 +247,8 @@ Fork a Repo Guide_>`_ and is well worth reading.
Pull request against `saltstack/salt`_ are automatically tested on a
variety of operating systems and configurations. On average these tests
take 30 minutes. Depending on your GitHub notification settings you may
also receive an email message about the test results.
take a couple of hours. Depending on your GitHub notification settings
you may also receive an email message about the test results.
Test progress and results can be found at http://jenkins.saltstack.com/.
@ -219,209 +257,34 @@ Fork a Repo Guide_>`_ and is well worth reading.
Salt's Branch Topology
----------------------
There are three different kinds of branches in use: |repo_primary_branch|, main release
branches, and dot release branches.
- All feature work should go into the ``|repo_primary_branch|`` branch.
- Bug fixes and documentation changes should go into the oldest **supported
main** release branch affected by the the bug or documentation change (you
can use the blame button in github to figure out when the bug was introduced).
Supported releases are the last 2 releases. For example, if the latest release
is 2018.3, the last two release are 2018.3 and 2017.7.
Main release branches are named after a year and month, such as
``2016.11`` and ``2017.7``.
- Hot fixes, as determined by SaltStack's release team, should be submitted
against **dot** release branches. Dot release branches are named after a
year, month, and version. Examples include ``2016.11.8`` and ``2017.7.2``.
.. note::
GitHub will open pull requests against Salt's main branch, ``|repo_primary_branch|``,
by default. Be sure to check which branch is selected when creating the
pull request.
The |repo_primary_branch| Branch
================================
The ``|repo_primary_branch|`` branch is unstable and bleeding-edge. Pull requests containing
feature additions or non-bug-fix changes should be made against the ``|repo_primary_branch|``
branch.
.. note::
If you have a bug fix or documentation change and have already forked your
working branch from ``|repo_primary_branch|`` and do not know how to rebase your commits
against another branch, then submit it to ``|repo_primary_branch|`` anyway. SaltStack's
development team will be happy to back-port it to the correct branch.
**Please make sure you let the maintainers know that the pull request needs
to be back-ported.**
Main Release Branches
=====================
The current release branch is the most recent stable release. Pull requests
containing bug fixes or documentation changes should be made against the oldest supported main
release branch that is affected.
The branch name will be a date-based name such as ``2016.11``.
Bug fixes are made on this branch so that dot release branches can be cut from
the main release branch without introducing surprises and new features. This
approach maximizes stability.
Dot Release Branches
====================
Prior to tagging an official release, a branch will be created when the SaltStack
release team is ready to tag. The dot release branch is created from a main release
branch. The dot release branch will be the same name as the tag minus the ``v``.
For example, the ``2017.7.1`` dot release branch was created from the ``2017.7``
main release branch. The ``v2017.7.1`` release was tagged at the ``HEAD`` of the
``2017.7.1`` branch.
This branching strategy will allow for more stability when there is a need for
a re-tag during the testing phase of the release process and further increases
stability.
Once the dot release branch is created, the fixes required for a given release,
as determined by the SaltStack release team, will be added to this branch. All
commits in this branch will be merged forward into the main release branch as
well.
Merge Forward Process
=====================
The Salt repository follows a "Merge Forward" policy. The merge-forward
behavior means that changes submitted to older main release branches will
automatically be "merged-forward" into the newer branches.
For example, a pull request is merged into ``2017.7``. Then, the entire
``2017.7`` branch is merged-forward into the ``2018.3`` branch, and the
``2018.3`` branch is merged-forward into the ``|repo_primary_branch|`` branch.
This process makes is easy for contributors to make only one pull-request
against an older branch, but allows the change to propagate to all **main**
release branches.
The merge-forward work-flow applies to all main release branches and the
operation runs continuously.
Merge-Forwards for Dot Release Branches
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The merge-forward policy applies to dot release branches as well, but has a
slightly different behavior. If a change is submitted to a **dot** release
branch, the dot release branch will be merged into its parent **main**
release branch.
For example, a pull request is merged into the ``2017.7.2`` release branch.
Then, the entire ``2017.7.2`` branch is merged-forward into the ``2017.7``
branch. From there, the merge forward process continues as normal.
The only way in which dot release branches differ from main release branches
in regard to merge-forwards, is that once a dot release branch is created
from the main release branch, the dot release branch does not receive merge
forwards.
.. note::
The merge forward process for dot release branches is one-way:
dot release branch --> main release branch.
Salt uses a typical branch strategy - ``master`` is the next expected release.
Code should only make it to ``master`` once it's production ready. This means
that typical changes (fixes, features) should have accompanying tests.\
Closing GitHub issues from commits
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This "merge-forward" strategy requires that `the magic keywords to close a
GitHub issue <Closing issues via commit message_>`_ appear in the commit
message text directly. Only including the text in a pull request will not
close the issue.
GitHub will close the referenced issue once the *commit* containing the
magic text is merged into the default branch (``|repo_primary_branch|``). Any magic text
input only into the pull request description will not be seen at the
Git-level when those commits are merged-forward. In other words, only the
commits are merged-forward and not the pull request text.
SaltStack encourages using `the magic keywords to close a GitHub issue <Closing
issues via commit message_>`_. These should appear in the commit message text
directly.
.. _backporting-pull-requests:
Backporting Pull Requests
=========================
-------------------------
If a bug is fixed on ``|repo_primary_branch|`` and the bug is also present on a
currently-supported release branch, it will need to be back-ported to an
applicable branch.
.. note:: Most Salt contributors can skip these instructions
These instructions do not need to be read in order to contribute to the
Salt project! The SaltStack team will back-port fixes on behalf of
contributors in order to keep the contribution process easy.
These instructions are intended for frequent Salt contributors, advanced
Git users, SaltStack employees, or independent souls who wish to back-port
changes themselves.
It is often easiest to fix a bug on the oldest supported release branch and
then merge that branch forward into ``|repo_primary_branch|`` (as described earlier in this
document). When that is not possible the fix must be back-ported, or copied,
into any other affected branches.
These steps assume a pull request ``#1234`` has been merged into ``|repo_primary_branch|``.
And ``upstream`` is the name of the remote pointing to the main Salt repo.
#. Identify the oldest supported release branch that is affected by the bug.
#. Create a new branch for the back-port by reusing the same branch from the
original pull request.
Name the branch ``bp-<NNNN>`` and use the number of the original pull
request.
.. code-block:: bash
git fetch upstream refs/pull/1234/head:bp-1234
git checkout bp-1234
#. Find the parent commit of the original pull request.
The parent commit of the original pull request must be known in order to
rebase onto a release branch. The easiest way to find this is on GitHub.
Open the original pull request on GitHub and find the first commit in the
list of commits. Select and copy the SHA for that commit. The parent of
that commit can be specified by appending ``~1`` to the end.
#. Rebase the new branch on top of the release branch.
* ``<release-branch>`` is the branch identified in step #1.
* ``<orig-base>`` is the SHA identified in step #3 -- don't forget to add
``~1`` to the end!
.. code-block:: bash
git rebase --onto <release-branch> <orig-base> bp-1234
Note, release branches prior to ``2016.11`` will not be able to make use of
rebase and must use cherry-picking instead.
#. Push the back-port branch to GitHub and open a new pull request.
Opening a pull request for the back-port allows for the test suite and
normal code-review process.
.. code-block:: bash
git push -u origin bp-1234
On rare occasions, a serious bug will be found in the middle of a release
cycle. These bugs will require a point release. Contributors should still
submit fixes directly to ``master``, but they should also call attention to the
fact that it addresses a critical issue and will need to be back-ported.
Keeping Salt Forks in Sync
--------------------------
Salt advances quickly. It is therefore critical to pull upstream changes
from upstream into your fork on a regular basis. Nothing is worse than putting
hard work into a pull request only to see bunches of merge conflicts because it
has diverged too far from upstream.
Salt advances quickly. It is therefore critical to pull upstream changes from
upstream into your fork on a regular basis. Nothing is worse than putting hard
work into a pull request only to see bunches of merge conflicts because it has
diverged too far from upstream.
.. seealso:: `GitHub Fork a Repo Guide`_
@ -450,20 +313,20 @@ the name of the main `saltstack/salt`_ repository.
git fetch upstream
#. Update your copy of the ``|repo_primary_branch|`` branch.
#. Update your copy of the ``master`` branch.
.. code-block:: bash
git checkout |repo_primary_branch|
git merge --ff-only upstream/|repo_primary_branch|
git checkout master
git merge --ff-only upstream/master
If Git complains that a fast-forward merge is not possible, you have local
commits.
* Run ``git pull --rebase origin |repo_primary_branch|`` to rebase your changes on top of
* Run ``git pull --rebase origin master`` to rebase your changes on top of
the upstream changes.
* Or, run ``git branch <branch-name>`` to create a new branch with your
commits. You will then need to reset your ``|repo_primary_branch|`` branch before
commits. You will then need to reset your ``master`` branch before
updating it with the changes from upstream.
If Git complains that local files will be overwritten, you have changes to
@ -474,7 +337,7 @@ the name of the main `saltstack/salt`_ repository.
.. code-block:: bash
git push origin |repo_primary_branch|
git push origin master
#. Repeat the previous two steps for any other branches you work with, such as
the current release branch.
@ -505,28 +368,6 @@ If you do not wish to receive these notifications, please add your GitHub
handle to the blacklist line in the ``.mention-bot`` file located in the
root of the Salt repository.
.. _probot-gpg-verification:
GPG Verification
----------------
SaltStack has enabled `GPG Probot`_ to enforce GPG signatures for all
commits included in a Pull Request.
In order for the GPG verification status check to pass, *every* contributor in
the pull request must:
- Set up a GPG key on local machine
- Sign all commits in the pull request with key
- Link key with GitHub account
This applies to all commits in the pull request.
GitHub hosts a number of `help articles`_ for creating a GPG key, using the
GPG key with ``git`` locally, and linking the GPG key to your GitHub account.
Once these steps are completed, the commit signing verification will look like
the example in GitHub's `GPG Signature Verification feature announcement`_.
Bootstrap Script Changes
------------------------
@ -551,6 +392,13 @@ Script, see the Bootstrap Script's `Contributing Guidelines`_.
.. _GPG Probot: https://probot.github.io/apps/gpg/
.. _help articles: https://help.github.com/articles/signing-commits-with-gpg/
.. _GPG Signature Verification feature announcement: https://github.com/blog/2144-gpg-signature-verification
.. _bootstrap-salt.sh: https://github.com/saltstack/salt/blob/|repo_primary_branch|/salt/cloud/deploy/bootstrap-salt.sh
.. _bootstrap-salt.sh: https://github.com/saltstack/salt/blob/master/salt/cloud/deploy/bootstrap-salt.sh
.. _salt-bootstrap repo: https://github.com/saltstack/salt-bootstrap
.. _Contributing Guidelines: https://github.com/saltstack/salt-bootstrap/blob/develop/CONTRIBUTING.md
.. _`Black`: https://pypi.org/project/black/
.. _`SEP 15`: https://github.com/saltstack/salt-enhancement-proposals/pull/21
.. _`pre-commit`: https://pre-commit.com/
.. _`SaltStack Community Slack`: https://saltstackcommunity.herokuapp.com/
.. _`#salt on freenode`: http://webchat.freenode.net/?channels=salt&uio=Mj10cnVlJjk9dHJ1ZSYxMD10cnVl83
.. _`saltstack meetup`: https://www.meetup.com/pro/saltstack/
.. _`saltstack on serverfault`: https://serverfault.com/questions/tagged/saltstack

197
doc/topics/development/conventions/style.rst
View file

@ -4,17 +4,13 @@
Salt Coding Style
=================
Salt is developed with a certain coding style, while the style is dominantly
PEP 8 it is not completely PEP 8. It is also noteworthy that a few
development techniques are also employed which should be adhered to. In the
end, the code is made to be "Salty".
To make it easier to contribute and read Salt code, SaltStack has `adopted
Black <SEP 15_>`_ as its code formatter. There are a few places where Black is
silent, and this guide should be used in those cases.
Most importantly though, we will accept code that violates the coding style and
KINDLY ask the contributor to fix it, or go ahead and fix the code on behalf of
the contributor. Coding style is NEVER grounds to reject code contributions,
and is never grounds to talk down to another member of the community (There are
no grounds to treat others without respect, especially people working to
improve Salt)!!
Coding style is NEVER grounds to reject code contributions, and is never
grounds to talk down to another member of the community (There are no grounds
to treat others without respect, especially people working to improve Salt)!
.. _pylint-instructions:
@ -65,27 +61,6 @@ Multi-word variables should be separated by an underscore.
Variables which are two-letter words should have an underscore appended
to them to pad them to three characters.
Strings
=======
Salt follows a few rules when formatting strings:
Single Quotes
-------------
In Salt, all strings use single quotes unless there is a good reason not to.
This means that docstrings use single quotes, standard strings use single
quotes etc.:
.. code-block:: python
def foo():
'''
A function that does things
'''
name = 'A name'
return name
Formatting Strings
------------------
@ -104,31 +79,8 @@ Please do NOT use printf formatting.
Docstring Conventions
---------------------
Docstrings should always add a newline, docutils takes care of the new line and
it makes the code cleaner and more vertical:
`GOOD`:
.. code-block:: python
def bar():
'''
Here lies a docstring with a newline after the quotes and is the salty
way to handle it! Vertical code is the way to go!
'''
return
`BAD`:
.. code-block:: python
def baz():
'''This is not ok!'''
return
When adding a new function or state, where possible try to use a
``versionadded`` directive to denote when the function or state was added.
``versionadded`` directive to denote when the function, state, or parameter was added.
.. code-block:: python
@ -141,16 +93,13 @@ When adding a new function or state, where possible try to use a
msg : None
The string to be printed.
'''
print msg
print(msg)
If you are uncertain what version should be used, either consult a core
developer in IRC or bring this up when opening your
:ref:`pull request <installing-for-development>` and a core developer will add the proper
version once your pull request has been merged. Bugfixes will be available in a
bugfix release (i.e. 0.17.1, the first bugfix release for 0.17.0), while new
features are held for feature releases, and this will affect what version
number should be used in the ``versionadded`` directive.
developer in IRC or bring this up when opening your :ref:`pull request
<installing-for-development>` and a core developer will let you know what
version to add. Typically this will be the next element in the `periodic table
<https://en.wikipedia.org/wiki/List_of_chemical_elements>`_.
Similar to the above, when an existing function or state is modified (for
example, when an argument is added), then under the explanation of that new
@ -176,7 +125,7 @@ significantly, the ``versionchanged`` directive can be used to clarify this:
.. versionadded 0.17.0
'''
print 'Greetings! {0}\n\n{1}'.format(msg, signature)
print('Greetings! {0}\n\n{1}'.format(msg, signature))
Dictionaries
@ -257,130 +206,16 @@ avoided.
.. _`absolute imports`: http://legacy.python.org/dev/peps/pep-0328/#rationale-for-absolute-imports
Vertical is Better
==================
When writing Salt code, vertical code is generally preferred. This is not a hard
rule but more of a guideline. As PEP 8 specifies, Salt code should not exceed 79
characters on a line, but it is preferred to separate code out into more
newlines in some cases for better readability:
.. code-block:: python
import os
os.chmod(
os.path.join(self.opts['sock_dir'],
'minion_event_pub.ipc'),
448
)
Where there are more line breaks, this is also apparent when constructing a
function with many arguments, something very common in state functions for
instance:
.. code-block:: python
def managed(name,
source=None,
source_hash='',
user=None,
group=None,
mode=None,
template=None,
makedirs=False,
context=None,
replace=True,
defaults=None,
saltenv=None,
backup='',
**kwargs):
.. note::
Making function and class definitions vertical is only required if the
arguments are longer then 80 characters. Otherwise, the formatting is
optional and both are acceptable.
Line Length
-----------
For function definitions and function calls, Salt adheres to the PEP-8
specification of at most 80 characters per line.
Non function definitions or function calls, please adopt a soft limit of 120
characters per line. If breaking the line reduces the code readability, don't
break it. Still, try to avoid passing that 120 characters limit and remember,
**vertical is better... unless it isn't**
Indenting
=========
Some confusion exists in the python world about indenting things like function
calls, the above examples use 8 spaces when indenting comma-delimited
constructs.
The confusion arises because the pep8 program INCORRECTLY flags this as wrong,
where PEP 8, the document, cites only using 4 spaces here as wrong, as it
doesn't differentiate from a new indent level.
Right:
.. code-block:: python
def managed(name,
source=None,
source_hash='',
user=None)
WRONG:
.. code-block:: python
def managed(name,
source=None,
source_hash='',
user=None)
Lining up the indent is also correct:
.. code-block:: python
def managed(name,
source=None,
source_hash='',
user=None)
This also applies to function calls and other hanging indents.
pep8 and Flake8 (and, by extension, the vim plugin Syntastic) will complain
about the double indent for hanging indents. This is a `known conflict
<https://github.com/jcrocholl/pep8/issues/167#issuecomment-15936564>`_ between
pep8 (the script) and the actual PEP 8 standard. It is recommended that this
particular warning be ignored with the following lines in
``~/.config/flake8``:
.. code-block:: ini
[flake8]
ignore = E226,E241,E242,E126
Make sure your Flake8/pep8 are up to date. The first three errors are ignored
by default and are present here to keep the behavior the same. This will also
work for pep8 without the Flake8 wrapper -- just replace all instances of
'flake8' with 'pep8', including the filename.
Code Churn
==========
Many pull requests have been submitted that only churn code in the name of
PEP 8. Code churn is a leading source of bugs and is strongly discouraged.
PEP 8. Code churn is a leading source of bugs and is **strongly discouraged**.
While style fixes are encouraged they should be isolated to a single file per
commit, and the changes should be legitimate, if there are any questions about
whether a style change is legitimate please reference this document and the
official PEP 8 (http://legacy.python.org/dev/peps/pep-0008/) document before
changing code. Many claims that a change is PEP 8 have been invalid, please
double check before committing fixes.
.. _`SEP 15`: https://github.com/saltstack/salt-enhancement-proposals/pull/21