Add collection-meta.yaml linter #617
Conversation
src/antsibull/collection_meta.py
Outdated
| for collection_name, collection_data in data["collections"].items(): | ||
| self.data[collection_name] = CollectionMetadata(collection_data) | ||
| @staticmethod | ||
| def load_from(deps_dir: str | None): |
There was a problem hiding this comment.
| def load_from(deps_dir: str | None): | |
| def load_from(deps_dir: str | os.PathLike[str] | None): |
There was a problem hiding this comment.
We can also use StrPath here :)
| for collection in sorted(remaining_collections): | ||
| print(f"collections: No metadata present for {collection}") |
There was a problem hiding this comment.
Ooops, yes...
|
Thanks for addressing the feedback. I'll try to take a look later today or tomorrow now that I'm back from Labor Day travel. |
| else: | ||
| if self.reason_text is not None: |
There was a problem hiding this comment.
Can't this be a single elif?
There was a problem hiding this comment.
Technically it can be. But I find it easier to understand to first distinguish between the two cases other and not other, and inside these cases checking for specific conditions.
| alias="collection-directory", default=None | ||
| ) | ||
| repository: t.Optional[str] = None | ||
| tag_version_regex: t.Optional[str] = None |
There was a problem hiding this comment.
Note to self: pydantic can validate regexes if this is set to t.Optional[re.Pattern], but I think the other code still expects a string.
src/antsibull/collection_meta.py
Outdated
| for collection_name, collection_data in data["collections"].items(): | ||
| self.data[collection_name] = CollectionMetadata(collection_data) | ||
| @staticmethod | ||
| def load_from(deps_dir: StrPath | None): |
There was a problem hiding this comment.
Missing return type annotation
There was a problem hiding this comment.
Fixed in dd683e4.
src/antsibull/collection_meta.py
Outdated
| announce_version: t.Optional[PydanticPypiVersion] = None | ||
| new_name: t.Optional[str] = None | ||
| discussion: t.Optional[p.HttpUrl] = None | ||
| redirect_replacement_version: t.Optional[int] = None |
There was a problem hiding this comment.
I think the key should also have a _major_ in there. It's a different type than the other _version keys.
| redirect_replacement_version: t.Optional[int] = None | |
| redirect_replacement_major_version: t.Optional[int] = None |
There was a problem hiding this comment.
Meh, it's only announce_version that's a full version number. I guess version could also be changed to major_version? What do you think?
There was a problem hiding this comment.
👍 changed in 6b7271f.
Co-authored-by: Maxwell G <maxwell@gtmx.me>
Co-authored-by: Maxwell G <maxwell@gtmx.me>
Co-authored-by: Maxwell G <maxwell@gtmx.me>
|
Failing CI is caused by CiscoDevNet/intersight-ansible#137. |
|
Thanks for working on this and for applying all my feedback |
|
@gotmax23 thanks for reviewing this! |
Ensures that collection-meta.yaml has the right order, the right structure, and that all kind of conditions are right.
Assumes the format from ansible-community/ansible-build-data#450. That PR also fixes the order.
Note that this makes pydantic 2+ a required dependency, and thus also antsibull-core 3+.