Fighting markdownlint

[davidorme]

I have real reservations about having markdownlint in our pre-commit. We will have quite a variety of Markdown documents in our repo: plain markdown, but also Jupyter notebooks in Myst markdown (which might additionally be structured as slides, for example). Basically, I'm fighting it, rather than doing work:

markdownlint

(virtual-rainforest-Laomc1u4-py3.10) dorme@MacBook-Pro training % mdl development_toolchain.md
development_toolchain.md:82: MD013 Line length
development_toolchain.md:42: MD024 Multiple headers with the same content
development_toolchain.md:293: MD024 Multiple headers with the same content
development_toolchain.md:367: MD024 Multiple headers with the same content

Me

Yes, that's a long URL, I can't wrap it and yes, I know that heading is identical to the previous one, it is a slideshow! 😠

So, the current pre-commit hook tool (using the original Ruby implementation: markdownlint/markdownlint) is throwing errors that I want to ignore but it does not seem to be configurable at a line by line level in the same way that other tools are. You can create style files but you have to tell markdownlint to use it explicitly and I think these rule exceptions should be case by case, not global.

There is another derived implementation using Node.js at DavidAnson/markdownlint that seems to be popular and widely adopted. It supports these line by line rules:

<!-- markdownlint-disable MD024-->

...

<!-- markdownlint-disable MD013 -->
```bash
curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/get-poetry.py | python -
```
<!-- markdownlint-enable MD013 -->

That seems preferable to me - the downside is that it uses Node.js, but is swapping a Ruby developer requirement for Node.JS a problem?

[davidorme]

Also the Markdownlint extension to VSCode uses DavidAnson's implementation, so then we get consistent live linting in VSCode and in the pre-commit actions.

[alexdewar]

Those things seem like perfectly reasonable things for markdownlint to complain about 😉. Why do you need really long lines or multiple headings called the same thing....?

Totally up to you though. The other markdownlint looks useful. I don't think using it will mean developers have to install nodejs either -- I have a feeling pre-commit downloads it if needed.

[davidorme]

They are utterly reasonable things to complain about 😄 , but not allowing exceptions in a flexible way doesn't seem reasonable.

Long URLs

I can't see a way to fix this without being able to disable it for this line. I don't want to suppress long lines globally or even in this file. Just for this URL, so pre-commit passes.

## Workshop format

- Tricky to do worked examples in `git`
- Do ask questions!
- Slides are online:

<!-- markdownlint-disable-next-line MD013-->
[](https://github.com/ImperialCollegeLondon/virtual_rainforest/blob/main/source/development/training/git.ipynb)

Repeated headings

These headers are all the same because it is sub section from a slide show covering the doctest tool. I could change the headers for each of these slides to make them unique, but I would only be doing it to get markdownlint to shut up.

+++ {"slideshow": {"slide_type": "slide"}, "tags": []}

## The `doctest` package

- The **docstrings** for code often includes example use
- The `doctest` package runs those examples
- Check if documented example code is **correct**
  - Looks for inputs (`>>>`, `...`) and output
  - Typically simple, self-contained tests
  - **Exact match** to expected output
  - Look out for whitespace at the ends of lines!

+++ {"slideshow": {"slide_type": "subslide"}, "tags": []}

## The `doctest` package

```{code-cell} ipython3
# %load -s my_float_multiplier mfm.py
def my_float_multiplier(x: float, y: float) -> float:
    """Multiplies two floats together.

    Arguments:
        x: The first number
        y: The second number

    Examples:
        >>> my_float_multiplier(2.1, 3.6)
        7.56
    """

    return x * y
```

+++ {"slideshow": {"slide_type": "subslide"}, "tags": []}

## The `doctest` package

* Etc
* Etc
* You get the point

[alexdewar]

Hmm ok, I see your point! It is annoying having to contend with these things.

[dalonsoa]

Linters are supposed to make your life and that of future developers easier... in the long term. So if this is a nuisance, just scrap that check entirely or use the other tool you mention.

Having said that, after reviewing the documentation I have the feeling you are mixing two things in this repo:

• Contributing guidelines for the Virtual Rainforest tool
• Training material on software engineering best practices

The second is excellent - to be honest, it is really, really good! - but I don't think it should be part of this repo as it does not relate to Virtual Rainforest. What I would do is to separate that and put it into another repo on training material only - public, so everyone can benefit from it - and leave in this only the stuff directly related to the Virtual Rainforest. You can then tweak the linters appropriately for both repos, probably more loose for the training one.

[davidorme]

I completely agree that the training material is a bit out of place and I definitely think it could be moved into a different repo.

Having said that, it has been useful to put some more complex markdown documents (Myst notebooks, with code blocks and slide separators) through the main repo pre-commit process. We are going to be using documents like these in the public facing documentation (how-to documents particularly) so using them to shake down the settings and try out tools is valuable.

I'd like to hang on to them for the moment to test that ReadTheDocs is rendering correctly, but once we've got more 'proper' documentation, splitting them out will make a cleaner UX for the main project.

[davidorme]

Just to note, I've also added the mdformat tool and a couple of extensions to the poetry dev dependencies. It is basically black for markdown, and also quite closely aligned to the Myst flavour. It isn't quite there yet - there are some edge-cases, but I've been finding it useful.