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

Publish generated list of rules on documentation website #261

Merged
merged 12 commits into from
Aug 26, 2021
Merged

Publish generated list of rules on documentation website #261

merged 12 commits into from
Aug 26, 2021

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Aug 25, 2021

This will provide a convenient reference for information about the rules Arduino Lint applies to projects, including:

  • More verbose description of the rule than would be appropriate for the command line output.
  • Link to the reference information that will allow the reader to fully understand the cause of the rule violation and how to fix it.
  • Table showing the rule violation level (i.e., error/warning/disabled) vs. common tool configuration.

I added a bespoke "Rules > Introduction" page to give some general information about the rule system. , but the "Sketch", "Library", "Platform", and "Package index" pages are all auto-generated from the Arduino Lint source code and will be automatically updated any time rules are added or modified without any additional effort.

The rule documentation content is generated by the "ruledocsgen" Go program, which is a separate module, located in its own subfolder of the repository, similar to the standardized "docsgen" program that generates the command line reference. In order for the project infrastructure to support this supplemental module, I synced with the latest versions of the relevant "template" assets, which have had that very capability added since the time of the last sync.

The existing structure and content of the rule configurations was not sufficient for this new usage, so I adjusted and expanded on them:

  • Add dedicated field for reference URLs and add references to rules that did not already have one where appropriate
  • Add or expand the contents of the Description field.
    • This content is now written in Markdown. Its existing use is limited to the JSON format output. Since the Markdown syntax is very lightweight and intuitive, and any reader of the JSON format output will already have been presented with plenty of syntax, I don't think the change has a harmful effect on that output.

Demo: https://per1234.github.io/arduino-lint/dev/rules/

Resolves #158

In order to catch platform-specific bugs, the "Test Go" workflow uses a job matrix to run the tests on multiple runners.
The step that uploads code coverage data to Codecov is intended to run only during the Linux job. The runner name
`ubuntu-latest` was used in the conditional to accomplish this. However, it might become necessary or desirable to pin a
specific runner version (e.g., `ubuntu-18.04`). The accompanying adjustment to the conditional might be forgotten and
there would not be any obvious sign that the coverage upload had stopped, nor why.

This will be avoided by using the general `runner.os` context item to identify the Linux job in the conditional. This
will not be ideal in the event multiple Linux runners are added to the workflow's job matrix, but this is less likely to
occur and a redundant coverage data upload shouldn't cause any problems.
This projects contains two Go modules, one in the root and one in the `docsgen` subfolder. More will be added. In order
to support checks on the supplemental modules in the project subfolders, it's necessary to configure the commands to run
from their path. This is passed to the task via the GO_MODULE_PATH environment variable. If this variable is not defined,
the default root module path is used as default, preserving the previous task behavior.

The workflows use a job matrix to allow easy configuration for any number of module paths and a dedicated parallel job
for each module.
Many of the rule messages contain a URL that can be visited to find the detailed information about the problem and how
to fix it.

Moving this information to a dedicated field in the struct allows for improved formatting of the rule output as well as
enhanced capabilities for the use of this data in other applications.
Some of the content has been moved to a new URL. Although the visitor was redirected to the new location, meaning they
were not exactly broken, it is better to send them to the right location from the start.
@per1234 per1234 added topic: documentation Related to documentation for the project type: enhancement Proposed improvement topic: infrastructure Related to project infrastructure topic: code Related to content of the project itself labels Aug 25, 2021
@silvanocerza
Copy link
Contributor

What is the purpose of the Foo/Foo.ino file? 🤔

These will make it easy for users to find information about the problem that caused the rule violation, and how to solve
it.
@per1234
Copy link
Contributor Author

per1234 commented Aug 25, 2021

What is the purpose of the Foo/Foo.ino file?

No purpose whatsoever, it was an artifact from an experiment I accidentally committed to the repository. I have now removed it:
https://github.com/arduino/arduino-lint/compare/96310248eb0bc5d47e70e1e34cd4c405a31aaeae..7be0eb83432d5685e1f5e8df4a44c04f7d2fb63f
Thanks for catching that!

The `Description` field of the rule configuration was previously only used to add supplemental information to the verbose
tool output. In this usage, it did not need to stand on its own as a complete documentation of the rule, and also was not
a high priority since the verbose output is likely not as frequently used.

A new requirement for the rule configuration data has emerged from the project to publish a list of rules. The
`MessageTemplate` data is not suitable for use in this application due to being written for use in indicating a problem
that was found, rather than a documentation of the rule. The new requirement is compatible with the existing use of the
`Description` field, so it is not necessary to add a new field. However, the existing content of the `Description field
is mostly not inadequate, so it must be expanded.

This content is now written in Markdown. Its existing use is limited to the JSON format output. Since the Markdown syntax
is very lightweight and intuitive, and any reader of the JSON format output will already have been presented with plenty
of syntax, I don't think the change has a harmful effect on that output.
The tool output and rules list assume that the text in the required rule configuration fields is present. In cases where
it is left empty, this assumption might result in strange or misformatted output. At the moment they are all populated,
so that assumption is fine. In the event the test catches an empty field, it can either be fixed or the code changed to
not rely on that field being populated.
This will provide a convenient reference for information about the rules Arduino Lint applies to projects.

- More verbose description of the rule than would be appropriate for the command line output.
- Link to the reference information that will allow the reader to fully understand the cause of the rule violation and
  how to fix it.
- Table showing the rule violation level (i.e., error/warning/disabled) vs. common tool configuration.

I added a bespoke "Rules > Introduction" page to give some general information about the rule system. , but the "Sketch",
"Library", "Platform", and "Package index" pages are all auto-generated from the Arduino Lint source code and will be
automatically updated any time rules are added or modified without any additional effort.

The rule documentation content is generated by the "ruledocsgen" Go program, which is a separate module, located in its
own subfolder of the repository, similar to the standardized "docsgen" program that generates the command line reference.
…iptions

Some of the rules must refer to data paths in configuration files such as the package index. In the case of arrays, it is
necessary to indicate that the reference applies to an arbitrary element. Prefiously, the `[]` syntax was used for this
purpose (e.g., `packages[].tools[]`). It is now changed to using `[*]` (e.g., `packages[*].tools[*]`).
The primary purpose of text of the rule configuration's `Description` field is for display in the rule reference section
of the documentation website. For this reason, it is written in Markdown, and thus can be output as is.

The situation is different with the rule configuration's `Brief` field, since it is displayed prominently in the tool
output. For this reason, the use of Markdown would not be appropriate. This text may contain incidental markup characters
that would result in unwanted formatting and thus the `Brief` field text must be escaped for display on the website.
…nner

There is a convention of configuring Arduino boards platforms to provide a user interface for configuring the build
process through the addition of references and fallback empty definitions of a series of properties that follow the
format of `compiler.<pattern type>.extra_flags`, where "<pattern type>" corresponds to the compilation pattern that
references them:

- `c`
- `cpp`
- `S`
- `ar`
- `c.elf`

Previously, `x` was used as a placeholder for the component of the property name that varies in order to allow a single
rule to refer to the entire set. "<pattern type>" is a little bit more clear than "x".
…rule messages

Feedback was provided that the previous "additional" term was not very clear. "Unrecognized" was determined to be better.
@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2021

Codecov Report

Merging #261 (ff3b56a) into main (1af853e) will decrease coverage by 0.14%.
The diff coverage is 84.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #261      +/-   ##
==========================================
- Coverage   89.83%   89.69%   -0.15%     
==========================================
  Files          43       44       +1     
  Lines        6327     6492     +165     
==========================================
+ Hits         5684     5823     +139     
- Misses        531      548      +17     
- Partials      112      121       +9     
Flag Coverage Δ
unit 89.69% <84.24%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ternal/rule/ruleconfiguration/ruleconfiguration.go 100.00% <ø> (ø)
internal/rule/rulefunction/platform.go 96.65% <ø> (ø)
ruledocsgen/main.go 83.95% <83.95%> (ø)
internal/result/result.go 91.44% <100.00%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1af853e...ff3b56a. Read the comment docs.

@per1234
Copy link
Contributor Author

per1234 commented Aug 26, 2021

Hi @silvanocerza. I sent you another review request because I had to make a code change to this PR. There is a pretty significant diff for the full force push:
https://github.com/arduino/arduino-lint/compare/c6e7e9360a0ce836414081a4598f4d3ccb306215..ff3b56ae71ba98c39c44ccf3bd1f1dffdc2f0618
but this is because it is making all the improvements that resulted from @ubidefeo's excellent review of the content.

The code changes of interest are to escape the rule "brief" text to make it suitable for inclusion in the Markdown source documents the website is generated from:
f62a372

Copy link
Contributor

@ubidefeo ubidefeo left a comment

Choose a reason for hiding this comment

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

Went through all the agreed upon changes to this and found everything properly adjusted.
Fantastic job

@per1234 per1234 merged commit f2a37a7 into arduino:main Aug 26, 2021
@per1234 per1234 deleted the rule-docs branch August 26, 2021 22:30
@per1234 per1234 self-assigned this Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself topic: documentation Related to documentation for the project topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Where can I find the list of rules?
4 participants