Skip to content

Latest commit

 

History

History
174 lines (144 loc) · 7.81 KB

Contributing.adoc

File metadata and controls

174 lines (144 loc) · 7.81 KB
Raw

Contributing

If you’re reading this, hopefully you are considering helping out with Ansible Agnostic Deployer aka AgnosticD.

These are our contribution guidelines for helping out with the project. Any suggestions, improvements, clarifications etc., please let us know via a GitHub issue.

General rules

  • The Ansible Code of Conduct still applies.

  • For git messages, branch names, etc., follow Git Style Guide.

  • Pull Requests should reference an issue whenever possible.

  • Pull Requests must contain a well crafted description.

    • In your Pull Request, specify closes #NUM to automatically close the issue when the PR is merged.

  • To contribute, fork and make a pull request against the development branch.

  • Use asciidoc for documentation.

  • Pull requests should only change:

    • One role

    • One config

    • If more than one, please explain why an exception should apply. Ask yourself, « Can this pull request be separated in several smaller pull requests ? »

  • Pull Request titles:

    • Must start with Add | Change | Fix | Remove

    • followed by config | workload | role | core | test | documentation

    • followed by the name of the config, workload, role, cloud provider, test or documentation

    • followed by a description that explains in sufficient details the why and what.

      • Leaving this empty or not putting any effort into the description will lead to the PR being sent back or closed.

  • Pull Request must be tested. Explain how you tested your change.

    • For example, you can state that you tested the changes against config X on cloud provider Y.

  • Owner of existing role or config must be set as reviewer.

    • If new role or config, owner must be identified.

  • If your Pull Request is not ready for review, open it as Draft or prefix with WIP in the title.

  • AgnosticD is part of the Red Hat Community of Practices; the Red Hat CoP Contribution Guidelines apply.

  • Do not push binary files or large files into Git. Instead, expose them, for example using public Object storage like S3, and fetch them with ansible using modules like get_url.

  • Destroy playbooks must be idem-potent. If run twice, they should not exit with an error.

Code Quality Rules

  1. A YAML .yamllint file should be added to every role and config when any substantial change is applied to the role or config, all new roles and configs must include a .yamllint. The AgnosticD standard .yamllint configuration is shown below. See also Official yamllint documentation.

    extends: default
rules:
  comments:
    require-starting-space: false
  comments-indentation: disable
  indentation:
    indent-sequences: consistent
  line-length:
    max: 120
    allow-non-breakable-inline-mappings: true

  2. All tasks should be in YAML literal. No foo=bar inline notation. See here.

  3. Indentation is 2 white-spaces.

  4. No in-line JSON format in Ansible

  5. Avoid the use of command or shell module when specific Ansible module exists to do the same. If you must, please explain why.

    • If you have to, prefer command module to shell module when possible.

    • Use k8s and k8s_info modules as much as possible and not oc command.

  6. All tasks AND plays must have names.

  7. Roles must have documented variables. This can be documented in either:

    • Role defaults/main.yml (preferred)

    • Role README file

    • Choose one or the other. If you are commenting your variables in the defaults, your README should just point to this file. Do not split your variable documentation across these.

  8. Roles must have meta/main.yml with author/contact information for support.

  9. Configs must have documented variables. This can be documented in either:

    • Config default_vars*.yml (preferred)

    • Config README file

    • Choose one or the other. If you are commenting your variables in the default_vars.yml, your README should just point to this file. Do not split your variable documentation across these.

  10. Be extra careful with external dependencies. Identify them and make sure the versions are pinned (use versions, tags, commit ID, etc.).

    • External Git repos

    • Libraries/Modules

    • Containers

  11. In a role, ensure all variables names are prefixed with the role name to avoid name collisions with other roles.

  12. Do not add ignore_errors to a task without justification. Prefer use of failed_when if a condition is not an error.

About reviewing Pull Requests

  1. Do not merge a PR after reviewing unless explicitely asked for

    • Approval and merging are different.

    • Other people might be reviewing at the same time, or want to review that PR

    • The PR might not be fully tested by the author yet

  2. If a specific person was requested for review, don’t merge before that person reviewed, or before the request was canceled.

  3. Take your time. If your PR is merged tomorrow instead of today, is it a big deal?

  4. Pull request for urgent critical fix for production must be titled and labeled accordingly.

    Example
    URGENT Fix config ansible-tower, update Windows AMI Ids for all regions

Those images have been deleted and are not available anymore.
This change, if applied, will update the AMI Ids in 'foobar' config for all region.

closes #1234

labels: bug,urgent

  5. Please use labels to categorize Pull Requests and Issues.

Ansible rules

# This
- name: Create a directory
  file:
    state: directory
    path: /tmp/deletethis

# Not this
- name: Create a directory
  file: state=directory path=/tmpt/deletethis

  • Module arguments should be indented two spaces

# This
- name: Create a directory
  file:
    state: directory
    path: /tmp/deletethis

# Not This
- name: Create a directory
  file:
      state: directory
      path: /tmp/deletethis

  • There should be a single line break between tasks

  • Tags should be in multi-line format and indented two spaces just like module arguments above

# This
- name: "Check hosts.equiv"
  stat:
    path: /etc/hosts.equiv
  register: hosts_equiv_audit
  always_run: yes
  tags:
    - tag1
    - tag2

# Not This
- name: "Check hosts.equiv"
  stat:
    path: /etc/hosts.equiv
  register: hosts_equiv_audit
  always_run: yes
  tags: [tag1,tag2]

  • Every task must be named and provide brief descriptions about the task being accomplished.

Git

Please follow the Git Style Guide.

Note: during the review process, you may add new commits to address review comments or change existing commits. However, before getting your PR merged, please squash commits to a minimum set of meaningful commits. This can be done directly in the github web UI.

If you’ve broken your work up into a set of sequential changes and each commit pass the tests on their own then that’s fine. If you’ve got commits fixing typos or other problems introduced by previous commits in the same PR, then those should be squashed before merging.