Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Integrate Vale style checker #116

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

beshleman
Copy link
Contributor

@beshleman beshleman commented Jul 30, 2021

Overview

This PR does two things: 1) Vale style checking, and 2) fixes errors reported by the checker.

It does not fix warnings and suggestions because there are over 2000 of them.

Where are the style checker rules?

This PR introduces the style checker via a Github Action maintained in this repository. The action pulls down the style rules, a configuration file, and the Vale executable. It then uses Vale to lint the file paths passed to it.

The bulk of the rules may be found in styles/gitlab. GitLab actively develops their Vale-compliant rules and it seemed wise to not reinvent the wheel. We may pick and choose the rules we like and discard the rest. Please share your opinions on the rules that you see in styles/gitlab!

Custom rules specific to Vates documentation (specialized vocabulary and proper nouns) can be found in styles/vates.

Is the style reusable?

The rules are kept in a separate repository so that the rules can be used across projects. The style-check script and the GitHub Action are meant to ease the use of the styles by other projects.

The simplest usage is to have the style files in your code repository, but that makes sharing harder. The next best option is to have the styles be a repository and to use it in projects as a git submodule... but what if you only want to lint your README.md? Is it worth introducing a submodule just for linting? For some projects, that may seem less than ideal. Offering an alternative and more lightweight approach seemed necessary.

As mentioned above, the linter may be activated via shell and via GitHub actions.

shell

The following call checks all markdown files in docs/ with the style rules v0.2. If vale is not available, it is automatically downloaded (though not installed on PATH).

curl -sL "https://raw.githubusercontent.com/xcp-ng/vale-styles/v0.1/style-check" | bash -s v0.1 -- docs/

GitHub Action

See the file .github/workflows/vale.yml for how the action is used.

The GitHub action displays the style violations cleanly:

image

If Vale finds an error, then the CI pipeline fails. If it finds zero errors but some warnings/suggestions, then the CI pipeline passes.

See the CI run of this repository.

The Style Changes

Why so many back ticks?

You might see that words that were previously expressed in plaintext now are wrapped in back ticks to form inline code formatting. These instances are mostly technical terms often seen on a command line. That includes commands, files, and configuration file snippets. Sometimes they are package names or application names. It is not always clear which is best fitting.

For example, should we say chroot (inline) or chroot (no inline)? Both seem acceptable, although it seems that their usage is different. chroot seems to refer to a tool or a command, while chroot (no inline) refers to a concept. This PR tries to strike a balance. Any word that is not inlined is spell checked by Vale. Any uncommon jargon must be canonized in a "ignore list" of
words. So another rule of the thumb that was applied was number of occurrences. For example, glibc occurred only twice, so it seemed appropriate to wrap it in back ticks instead of add yet another entry to the spell checker. chroot, however, occurred many times and so seemed to deserve an entry in the spell checker.

Other Style changes

All small errors: typos, capitalization, spacing, etc...

Future

We expect the rule set to be modified heavily. Some things may seem more reasonable than others. This PR serves as a trial run for the tool and a starting point for a style.

Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
Including:
* converting **bolding** to `inline code` for menu options, parameters,
  etc…
* Converting abbreviation tx to TX.
* Capitalize Xen.
* Use inline code to highlight configuration parameters (such as
  `ethernet`).
* Double spacing
* Expand acronyms
* Capitalize acronyms in prose

Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
@beshleman beshleman requested a review from Wescoeur July 30, 2021 22:32
@beshleman beshleman changed the title Integrate Vale style checker WIP Integrate Vale style checker Jul 30, 2021
@beshleman beshleman changed the title WIP Integrate Vale style checker WIP: Integrate Vale style checker Jul 30, 2021
Signed-off-by: Bobby Eshleman <[email protected]>
Signed-off-by: Bobby Eshleman <[email protected]>
@beshleman beshleman changed the title WIP: Integrate Vale style checker Integrate Vale style checker Jul 30, 2021
docs/guides.md Show resolved Hide resolved
docs/guides.md.orig Show resolved Hide resolved
docs/migratetoxcpng.md.orig Show resolved Hide resolved
docs/cli_reference.md Show resolved Hide resolved
docs/cli_reference.md Show resolved Hide resolved
docs/cli_reference.md Show resolved Hide resolved
docs/cli_reference.md Show resolved Hide resolved
docs/cli_reference.md Show resolved Hide resolved
@@ -4170,7 +4170,7 @@ We advise to use Xen Orchestra instead of this method. See [Xen Orchestra rollin

Commands for controlling VM scheduled snapshots and their attributes.

The vmss objects can be listed with the standard object listing command (`xe vmss-list`), and the parameters manipulated with the standard parameter commands. For more information, see [Low-level parameter commands](#low-level-parameter-commands)
The `vmss` objects can be listed with the standard object listing command (`xe vmss-list`), and the parameters manipulated with the standard parameter commands. For more information, see [Low-level parameter commands](#low-level-parameter-commands)
Copy link
Member

Choose a reason for hiding this comment

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

Here's it's more a concept than a command, so either in caps or as an accepted word? By the way I'm all for adding accepted words for every concept that is met in the docs, rather than enforcing backticks each time we meet them. I don't think we'd lose much in quality and this would make contributions easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could be on board with that. I think it's slightly more pleasing to the eye when these types of things are called out by the backtick style, but I agree with your point that not requiring them to be wrapped with backticks will lower contribution overhead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we add all XAPI classes as accepted words?

Copy link
Member

Choose a reason for hiding this comment

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

Probably.

docs/troubleshooting.md.orig Show resolved Hide resolved
Comment on lines +17 to +38
<<<<<<< HEAD
## From Xen on Linux

If you are running Xen on your usual distro (Debian, Ubuntu…), you are using `xl` to manage your VMs, and also plain text configuration files. You can migrate to an existing XCP-ng host thanks to [this Python script](http://www-archive.xenproject.org/files/xva/xva.py).

1. Get that script in your current `dom0`.
1. Shutdown your VM
1. Run the script, VM by VM with for example: `./xva.py -c /etc/xen/vm1.cfg -n vm1 -s xcp_host_1 --username=root --password="mypassword" --no-ssl`. You can use a hostname or the IP address of your XCP-ng host (name `xcp_host_1` here)
1. Your disks are streamed while the configuration file is "translated" to a VM object in your XCP-ng host.
1. As soon it's done, you should be able to boot your VM on destination
1. Repeat for all your VMs

If you have an error telling you that you don't have an default SR, please choose a default SR on your XCP-ng pool (in XO, Home/Storage, hover on the storage you want to put by default, there's an icon for it).

:::warning
This script is a bit old and not tested since while. If you have issues, feel free to report that!
:::

## From Virtualbox
=======
## From VirtualBox
>>>>>>> 2ad9372 (docs/migratetoxcpng.md: style cleanup)
Copy link
Member

Choose a reason for hiding this comment

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

Bad rebase? 😅

Copy link
Contributor Author

@beshleman beshleman Aug 3, 2021

Choose a reason for hiding this comment

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

Lol yeah, let me change this to a WIP because I have that and a few other bad artifacts fixed locally. Just want to double check it is all cleaned up before pushing v2. One of the diff tools creating a .orig + plus I must have called git add docs/ or something clumsy like that.

@beshleman beshleman changed the title Integrate Vale style checker WIP: Integrate Vale style checker Aug 3, 2021

:::warning
Many guides on the internet for pfSense in Xen VMs will tell you to uncheck checksum options in the pfSense web UI, or to also disable RX offload on the Xen side. These are not only unnecessary, but some of them will make performance worse.
:::
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:::
:::


5. Checking the output
`# xe vm-param-list uuid=<VM_UUID> | grep other-config`
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Rebase leftovers


It is important that each RAID array has a different name as the system will not allow you to create a RAID array with the name of one that already exists. Normally, you would just continue on with different RAID device names such as `md1`, `md2`, `md3`, etc. It is also important to use different names for each storage repository such as "RAID storage", "RAID storage 2" and so on.
=======
>>>>>>> 046cf3b (docs/guides.md: remove trailing whitespace)
Copy link
Contributor

Choose a reason for hiding this comment

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

Rebase leftovers


We got a dedicated section on [how to migrate from Citrix Hypervisor to XCP-ng](upgrade.md#upgrade-from-xenserver).

<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Rebase leftovers

It should be empty, but if you have the bug, you got `check_update`.

Remove `/var/lib/xcp-ng-xapi-plugins/updater.py.lock` and that should fix it.
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Rebase leftovers

Comment on lines +582 to +583
=======
>>>>>>> 7c2ffe8 (docs/troubleshooting.md: style fixes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Rebase leftovers

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.

5 participants