coding guidelines

[kitzbergerg]

As part of #60 I want to enable all clippy lints that seem useful.

This also includes adding documentation to all methods and modules in the backend.

After all clippy lints are active there might not even be a need to write additional coding guidelines (as clippy can be quite strict).

[kitzbergerg]

@markus2330 Please have a lot at the metadata I added to the toml file. I put in there what clippy wanted and tried to piece together what I think would make sense.

[markus2330]

Amazing investigation! Really like the many lints ❤️

Only two very minor points, see comments.

Some of them might be too much, e.g. partial_pub_fields, but lets see if we hit them.

[kitzbergerg]

I'm kind of finished with what I can do in this PR for now (not yet to be merged).

I documented where I could in code and left TODOs where I didn't really know what to write (e.g. struct fields that represent the database). For that I would need some good documentation or some help to tell me what the individual fields are actually for (maybe discuss that on Tuesday or somebody helps me after the meeting).

Then there is still #![allow(clippy::module_name_repetitions)]. I don't actually know whether we want that, but I can't really do anything about that currently as I'm kind of blocked from renaming file by #48 (we did some renaming there and that would result in a huge merge conflict mess).

[kitzbergerg]

@markus2330 To view the docs type cargo doc --open and the generated html files should open in the browser. It's easier to look through code that way.

[markus2330]

Great job! ❤️

• please also add how to add documentation cargo doc --open to the documentation (README.md)
• struct fields that represent the database: Yes, good idea to document it here. Define database layout for plant hierarchy #48 and others can add documentation there.
• module_name_repetitions: I am all for not repeating module names. Not having to do that, is one of the main reasons why modules exists.

[kitzbergerg]

@lukashartl Once this PR is merge you probably need to change CI. There should be an additional check whether documentation links are broken. The command for that is cargo doc.

@markus2330 It is possible to set a flag in rust to deny warnings (rust-lang/cargo#8424 (comment)). We might want to enable that.

[markus2330]

@lukashartl Once this PR is merge you probably need to change CI. There should be an additional check whether documentation links are broken. The command for that is cargo doc.

@markus2330 It is possible to set a flag in rust to deny warnings (rust-lang/cargo#8424 (comment)). We might want to enable that.

I created #98 to track this, the deployment is higher priority though.

[markus2330]

Great work! Now please concentrate on documentation, architecture, big picture of backend etc., so that we can finish the first milestone without bigger merge conflicts.

[kitzbergerg]

Great work! Now please concentrate on documentation, architecture, big picture of backend etc., so that we can finish the first milestone without bigger merge conflicts.

There are still a few smaller issues open. I will quickly fix them and afterwards start with the doc/decisions.