yaml linter

[ericLemanissier]

We should consider linting conandata.yml and config.yml files, for example using yamllint
Example of case where it would have save some non negligible debugging time: conan-io/conan-center-index@61eb407#diff-c72439ef9b3281fe119686121ba38bb839dc40459798c2641142eb4135f15f61R16

exemple of execution with yamllint:
test.yml:

patches:
  a
  b
patches:
  c

$ yamllint --no-warnings test.yml
test.yml
  4:1       error    duplication of key "patches" in mapping  (key-duplicates)

[Croydon]

I like the idea, but the problem with this is that the CCI hooks don't require any other Python package so far and requiring dependencies makes handling them more complex

[uilianries]

I like the idea, but the problem with this is that the CCI hooks don't require any other Python package so far and requiring dependencies makes handling them more complex

True.

Also, we avoided linter because not all contributors know Python, I believe most people are C/C++ devs. So if we start to block because of linters, probably more users will give up first PR.

I would consider it as a recommendation in Docs, and of course, we can implement yaml linter hook, but not mandatory for CCI. I totally understand the trade off situation, we could avoid more errors in CCI.

[ericLemanissier]

I like the idea, but the problem with this is that the CCI hooks don't require any other Python package so far and requiring dependencies makes handling them more complex

Yes, this bothers me too. I searched for another linter directly callable from python code, but could not find one. I still posted the issue because maybe someone here knows one ?

Also, we avoided linter because not all contributors know Python, I believe most people are C/C++ devs. So if we start to block because of linters, probably more users will give up first PR.

I would consider it as a recommendation in Docs, and of course, we can implement yaml linter hook, but not mandatory for CCI. I totally understand the trade off situation, we could avoid more errors in CCI.

Just to be clear, the purpose is not to catch style errors, nor smelly code, but real errors with zero false positives.
e.g.: if someone adds a "patch" key to a config.yml which already has one, the net result is that all the patches mentioned in the original key are silently ignored. (which is what happened in conan-io/conan-center-index@61eb407#diff-c72439ef9b3281fe119686121ba38bb839dc40459798c2641142eb4135f15f61R16)

[uilianries]

Fair enough. I'll bring this issue to the CCI team. I'm a big fan of automation, so if we can reduce more errors adapting it, this would be cool.

[uilianries]

After talking to Conan team, the decision was using yaml linter internally in Jenkins, not as a hook. We can update the Docs explaining about yamllinter and internal check. The point is, it's interesting adding it, but not forcing conan-center hook with an external dependency, so the idea is adding into CDT Jenkins images and run before or after Conan export.

[ericLemanissier]

One new case today: conan-io/conan-center-index#6214 (review)

This is the "lucky" version, because it makes the build crash

ERROR: djinni-support-lib/1.0.0: Error in source() method, line 118
	tools.get(**self.conan_data["sources"][self.version], strip_root=True, destination=self._source_subfolder)
	KeyError: 'sources'

[uilianries]

Thank you for reporting!

[a4z]

This is a nice!
Thanks for cross referencing my PR so I see that
I was testing the yaml, but the default python yaml just loaded the file fine.... while yaml lint would have cached that.
Will add that to my tool box