Add MegaLinter #19

jtxa · 2022-11-02T10:29:51Z

Adds a GitHub workflow for MegaLinter with initial configuration and some fixed.

Successful linters:

Bash: bash-exec, shellcheck, shfmt
Editorconfig
JSON: eslint-plugin-jsonc, jsonlint, prettier, v8r
Markdown: markdownlint, markdown-link-check, markdown-table-formatter
Repository: gitleaks, git_diff, secretlint
Spelling: cspell, misspell
YAML: prettier, v8r, yamllint

Switched to be non-blocking:

CPP: cpplint - out of scope of this PR; changes would need more detailed reviews
HTML: djlint, htmllint - out of scope of this PR; files in git have troff commands and rest is generated

.editorconfig

README.md

jtxa · 2022-11-02T10:50:16Z

I kept the shell script reformatting in separate commits from the changes (for now).
For an easier review to decide if the format is now ok (I used some configuration values to match existing formatting as close as possible).

jtxa · 2022-11-02T10:51:27Z

@sierrafoxtrot Please have a look at the questions above. If you tell me your preference, I can make this PR ready for merge.

sierrafoxtrot · 2022-11-03T01:43:30Z

Adds a GitHub workflow for MegaLinter with initial configuration and some fixed.

Successful linters:
* JSON: eslint-plugin-jsonc, jsonlint, prettier, v8r

Curious about this one. We don't have any JSON or JS from memory. Does this cover something I'm forgetting?

jtxa · 2022-11-03T01:52:48Z

The new cSpell configuration is currently a JSON file.
But I take a look to convert it to YAML, that way comments can also be added and it might look a bit easier.

sierrafoxtrot · 2022-11-03T02:18:03Z

The new cSpell configuration is currently a JSON file. But I take a look to convert it to YAML, that way comments can also be added and it might look a bit easier.

I understand. This work is a huge leap forward Josef so happy to keep it as-is if that is the simplest way forward.

jtxa · 2022-11-03T19:52:22Z

A little polishing done here and there since the first push. The major ones are:

cSpell configuration as YAML: better readability and comments added why some settings were made.
All YAML files with indent of 2, everything else looks weird because of the arrays using - .
I did check if all the output of the doc scripts is the same as before, and now they are (see doc: Useless ref-index.so #21).

From my side this PR is ready now.

jtxa · 2022-11-04T00:03:20Z

Rebase to solve conflicts.
Also deleted shell/awk/ignore files, obsolete after #22

sierrafoxtrot

Thanks Josef, this is a huge improvement and significantly cleans things up. One query above but I'm keen to get this merged.

test/00/t0003a.sh

marcows · 2022-11-04T09:59:05Z

doc/CMakeLists.txt

+            # Order of preprocessors may be important
+            -t                     # Preprocess with tbl
+            -s                     # Preprocess with soelim, with search paths:
+            -I ${DocSourcePath}


Adding new and moving existing options shouldn't be mixed with style changes.

Yes that's true. I tried to kept the numbers of commits low for Scott ;-)

marcows · 2022-11-04T09:59:47Z

doc/CMakeLists.txt

+            -man                   # Macro Package man
+            # Order of preprocessors may be important
+            -t                     # Preprocess with tbl
+            -s                     # Preprocess with soelim, with search paths:


You write the order may be important, but swap it here in this commit which is mostly about style changes.

I don't know groff, and found no hint about this behavior in the options description in the man page.

But checking the output for being identical I recognized that the order of -P-I${image_stem_name} had an influence on the filename of the generated png.

I did now normalize the order of all groff calls. They order in general seems to be irrelevant, just that one is not.
I left the comment in the code, so the people are aware to be sure to check the output files if those things are changed.

Yep, the ordering is important except when it isn't :-(. Adding the image stem option was fiddly. Needed to give unique website images but also to support concurrent doc builds (there was a race condition around generating an image for one doc entry being overwritten by another running in parallel). The other main gotchas I encountered were around the args being passed to the preprocesors like soelim and tbl.

Add initial configuration with some checks disabled to be non-blocking: CPP_CPPLINT: Necessary code changes need to be reviewed in detail. HTML_DJLINT and HTML_HTMLHINT: The HTML files are preprocessed and contain some control commands. They cannot checked as is.

cSpell is used in Code and and can be checked on command line. It's also included in the MegaLinter workflow.

Although a valid alternative, nibble is more commonly used and the majority of the code uses nibble already.

- Avoid copying it to build dir - Apply executable flags to all shell scripts. - Auto-format by `shfmt -p -i 4 -sr -fn -w *`

- Fixed problems reported by `shellcheck`. - Apply executable flags to all shell scripts. - Auto-format shell scripts by `shfmt -p -i 4 -sr -fn -w *`.

- Avoid too long lines - Add comments for some command line arguments - Avoid local variables with `CMAKE` prefix - Fix dependencies to scripts - Move forgotten awk script from `etc` to `script`

Synchronize the order of arguments for the different `groff` calls without changing the output. This shall make the real differences more obvious. Add an explicit `-s` which is implicitly activated by `-I`. Specify options `-T` and `-man` first, do preprocessing in order: `-t`, `-s`, `-P`. At least `-P-I${image_stem_name}` must be done last, otherwise the generated file names uses a default instead of the given stem.

jtxa · 2022-11-04T12:52:05Z

Rebased
Doc-CMake changes splitted to mulitple commits with revised content and messages
I reverted some of my ci.yml changes. There will be a follow-up PR with Windows and MacOS build added.
The rest is the same as before. I repeated my check that the generated documentation is still the same.

jtxa commented Nov 2, 2022

View reviewed changes

.editorconfig Outdated Show resolved Hide resolved

jtxa commented Nov 2, 2022

View reviewed changes

README.md Show resolved Hide resolved

jtxa mentioned this pull request Nov 2, 2022

General discussion about GitHub Workflows #8

Open

43 tasks

jtxa force-pushed the mega-linter branch from b47976d to 51caced Compare November 3, 2022 19:43

jtxa marked this pull request as ready for review November 3, 2022 19:52

jtxa changed the title ~~[WIP] Add MegaLinter~~ Add MegaLinter Nov 3, 2022

jtxa force-pushed the mega-linter branch from 51caced to d2ff456 Compare November 4, 2022 00:01

sierrafoxtrot reviewed Nov 4, 2022

View reviewed changes

test/00/t0003a.sh Show resolved Hide resolved

sierrafoxtrot mentioned this pull request Nov 4, 2022

Remove leftover scripts after ref-index.so removal #23

Merged

marcows reviewed Nov 4, 2022

View reviewed changes

jtxa added 11 commits November 4, 2022 12:59

Add cSpell configuration

5bc1a68

cSpell is used in Code and and can be checked on command line. It's also included in the MegaLinter workflow.

Fix spelling errors

4431ee8

Change spelling of nybbles to nibbles

2ccbcfd

Although a valid alternative, nibble is more commonly used and the majority of the code uses nibble already.

README: Simplify, fix formatting and spelling

f0b7032

Fit C source into line length limit

897b24a

test: Use test_prelude.sh from source dir

6061575

- Avoid copying it to build dir - Apply executable flags to all shell scripts. - Auto-format by `shfmt -p -i 4 -sr -fn -w *`

doc: Fix some issues in shell scripts

4fe1eb7

- Fixed problems reported by `shellcheck`. - Apply executable flags to all shell scripts. - Auto-format shell scripts by `shfmt -p -i 4 -sr -fn -w *`.

doc: Refactor CMake file

c294e3a

- Avoid too long lines - Add comments for some command line arguments - Avoid local variables with `CMAKE` prefix - Fix dependencies to scripts - Move forgotten awk script from `etc` to `script`

doc: Do not copy scripts to build directory

ecdf4df

jtxa force-pushed the mega-linter branch from d2ff456 to eb2c86a Compare November 4, 2022 12:46

sierrafoxtrot approved these changes Nov 4, 2022

View reviewed changes

sierrafoxtrot merged commit 25a3107 into sierrafoxtrot:master Nov 4, 2022

jtxa deleted the mega-linter branch November 4, 2022 23:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add MegaLinter #19

Add MegaLinter #19

jtxa commented Nov 2, 2022

jtxa commented Nov 2, 2022

jtxa commented Nov 2, 2022

sierrafoxtrot commented Nov 3, 2022

jtxa commented Nov 3, 2022

sierrafoxtrot commented Nov 3, 2022

jtxa commented Nov 3, 2022 •

edited

Loading

jtxa commented Nov 4, 2022

sierrafoxtrot left a comment

marcows Nov 4, 2022

jtxa Nov 4, 2022

marcows Nov 4, 2022

jtxa Nov 4, 2022

jtxa Nov 4, 2022

sierrafoxtrot Nov 4, 2022

jtxa commented Nov 4, 2022

Add MegaLinter #19

Add MegaLinter #19

Conversation

jtxa commented Nov 2, 2022

jtxa commented Nov 2, 2022

jtxa commented Nov 2, 2022

sierrafoxtrot commented Nov 3, 2022

jtxa commented Nov 3, 2022

sierrafoxtrot commented Nov 3, 2022

jtxa commented Nov 3, 2022 • edited Loading

jtxa commented Nov 4, 2022

sierrafoxtrot left a comment

Choose a reason for hiding this comment

marcows Nov 4, 2022

Choose a reason for hiding this comment

jtxa Nov 4, 2022

Choose a reason for hiding this comment

marcows Nov 4, 2022

Choose a reason for hiding this comment

jtxa Nov 4, 2022

Choose a reason for hiding this comment

jtxa Nov 4, 2022

Choose a reason for hiding this comment

sierrafoxtrot Nov 4, 2022

Choose a reason for hiding this comment

jtxa commented Nov 4, 2022

jtxa commented Nov 3, 2022 •

edited

Loading