Skip to content

Doc / Deployment with ansible: generic install#5464

Merged
ThomasWaldmann merged 2 commits intoborgbackup:masterfrom
Samuel-BF:patch-1
Nov 7, 2020
Merged

Doc / Deployment with ansible: generic install#5464
ThomasWaldmann merged 2 commits intoborgbackup:masterfrom
Samuel-BF:patch-1

Conversation

@Samuel-BF
Copy link
Contributor

I replaced the pacman module with the package module, so the snippet work for most linuxes, and not only arch.

I also switched state from latest to present : it is meant to preserve idempotency. This is a debattable change.

Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please change the commit comment, so it contains the reasoning about this change?

maybe also a short summary of that "debatable change" link and why you think your change is an improvement.

Comment on lines +126 to +127
Automate setting up an repository server with the user, group, folders and
permissions a Ansible playbook could be used. Keep in mind the playbook
uses the Arch Linux `pacman <https://www.archlinux.org/pacman/pacman.8.html>`_
package manager to install and keep borg up-to-date.
permissions a Ansible playbook could be used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not related to your change, but while we're at it:

parse error. can you fix so this to understandable, correct english?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is maybe this what is meant?:

To automate setting up a borg repository server an Ansible playbook could be used.

Using "package" ansible module make the ansible playbook able to run
against most unix OSes. Pacman module only works with Arch and derivatives.

Also : changing state from "latest" to "present". Ansible should not be
a way to keep your system up-to-date : it's a configuration management
system and, as such, should not change anything if neither the playbook
nor the machine changed its state (idempotency).

Reference : https://github.com/ansible/ansible-lint/blob/master/lib/ansiblelint/rules/PackageIsNotLatestRule.py#L24
@Samuel-BF
Copy link
Contributor Author

Done. The commit comment is now :

Doc / Deployment with ansible: pacman -> package

Using "package" ansible module make the ansible playbook able to run
against most unix OSes. Pacman module only works with Arch and derivatives.

Also : changing state from "latest" to "present". Ansible should not be
a way to keep your system up-to-date : it's a configuration management
system and, as such, should not change anything if neither the playbook
nor the machine changed its state (idempotency).

Reference : https://github.com/ansible/ansible-lint/blob/master/lib/ansiblelint/rules/PackageIsNotLatestRule.py#L24

About :

parse error. can you fix so this to understandable, correct english?

What part was this comment about ? I shortened the ansible introduction, guessing it was about this part (second commit). I'm not a native english speaker/writer, so I'm less accurate in finding this kind of errors :) .

@ThomasWaldmann ThomasWaldmann merged commit 122dfce into borgbackup:master Nov 7, 2020
@ThomasWaldmann
Copy link
Member

Thanks! Can you check if we need a backport to 1.1-maint branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants