oxsecurity / megalinter

🦙 MegaLinter analyzes 50 languages, 22 formats, 21 tooling formats, excessive copy-pastes, spelling mistakes and security issues in your repository sources with a GitHub Action, other CI tools or locally.
https://megalinter.io
GNU Affero General Public License v3.0
1.81k stars 215 forks source link

Rename .markdown-lint.json to .markdownlint.json #853

Closed Kurt-von-Laven closed 2 years ago

Kurt-von-Laven commented 2 years ago

Describe the bug It would be best to name the templates after their defaults upstream to maximize Mega-Linter's compatibility with the linters it wraps.

To Reproduce Steps to reproduce the behavior:

  1. Compare the upstream documentation to Mega-Linter's Markdownlint template.

Expected behavior No configuration changes are needed to switch between running Markdownlint directly and via Mega-Linter.

Additional context This will have the added benefit of causing Schemastore to automatically check both Mega-Linter's template and the templates of downstream projects. I can't think of a way to do this that wouldn't be a breaking change, so I am proposing this for inclusion in v5 at earliest.

nvuillam commented 2 years ago

We can consider wrong naming of markdownlint.json as a bug (probably inherited from Super-Linter) , and i don't think many people has overridden it, so maybe it's not necessary mandatory to wait for V5 to fix that ^^ And to be honest, i'd like a V5 to have breaking changes more shiny than renaming a default config file :p

Kurt-von-Laven commented 2 years ago

That seems wise. I since realized that Mega-Linter already set this precedent with past config template renames.

calexandre commented 2 years ago

@nvuillam would you consider to set the default value of MARKDOWN_MARKDOWNLINT_CONFIG_FILE to LINTER_DEFAULT as I mentioned in #875 ? I've gone through that route and was able to keep compatibility with markdownlint-cli and only had to change the .mega-linter.yaml configuration file... It is not ideal, but at least I kept compatibility with the linter.

If the LINTER_DEFAULT was the default value, then we would not need to change a thing.

nvuillam commented 2 years ago

@calexandre @Kurt-von-Laven V5 will have default config file name as .markdownlint.json , I suggest u keep using the workarounds until it is released :)

Kurt-von-Laven commented 2 years ago

To head off any confusion, v5 has been released, but this hasn't been fixed yet. Refer to:

I think this issue basically boils down to doing a global find-replace of both file contents and filenames.

@calexandre, I see the value of making LINTER_DEFAULT the default configuration file not only for Markdownlint, but for every linter. I could certainly believe there is some deeper consideration I am overlooking on that point, @nvuillam?

nvuillam commented 2 years ago

LINTER_DEFAULT is a legacy variable of Super-Linter, I'm not sure it's very wise to use it , as linters locally installed won't use it :/ I thnk that as we announced the renaming of markdown-lint.json into markdownlint.json we can apply it in a next minor release of V5 :)

nvuillam commented 2 years ago

Plz forget my remark about LINTER_DEFAULT, i made a mistake with LINTER_RULES_PATH :)

Kurt-von-Laven commented 2 years ago

If I understand correctly, ultimately, you would also support making LINTER_DEFAULT the default configuration file (and possibly eventually obviating it) for all linters?

calexandre commented 2 years ago

Just to be clear, this is my current .mega-linter.yml (i stripped most of the configuration):

# Configuration file for Mega-Linter
# See all available variables at https://nvuillam.github.io/mega-linter/configuration/ and in linters documentation
(...)
MARKDOWN_MARKDOWNLINT_CONFIG_FILE: LINTER_DEFAULT

With the above in mind, is there any other recommendation for using the linter's defaults? I don't agree that the default should be .markdownlint.json, as I use the yaml version...one could argue that the contrary also applies. Instead, I think we should follow the linter's specification regarding the defaults and delegate to the linter the management of its own defaults - in the case of the markdownlint linter it supports multiple default flavors that go from json to yaml (and probably others). With the above in mind I still think that the setting I'm using LINTER_DEFAULT should be the actual default (or at least something that replicates the same functionality), or am I missing something?

Kurt-von-Laven commented 2 years ago

Right now you do have to specify LINTER_DEFAULT manually as you have done. I agree that this should be the default, and I think @nvuillam does too if I interpreted his response correctly.

nvuillam commented 2 years ago

@calexandre I confirm @Kurt-von-Laven 's answer, you must specify MARKDOWN_MARKDOWNLINT_CONFIG_FILE: LINTER_DEFAULT in your configuration

An evolution that we could do is to make possible to define config_file_name as a list and not as an unique config file name. That way, the behavior would be :