Skip to content
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packit_service/worker/helpers/testing_farm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1510,6 +1510,24 @@ def _payload_rpmlint(self, distro: str, compose: str) -> dict:
}
return payload

# TODO: this placeholder would no longer be needed if all the other tests
# are defined in it. It requires splitting the check status for each plan
# and restarting individual plans (either from this shared or custom)
Comment on lines +1513 to +1515
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This TODO comment indicates a future refactoring or improvement. It's good to acknowledge, but for better tracking and to keep the codebase clean, consider creating a dedicated issue in your issue tracker for this task. This ensures the work is formally tracked and doesn't get lost as the codebase evolves.

@implements_fedora_ci_test("shared")
def _payload_shared(self, distro: str, compose: str) -> dict:
git_repo = "https://forge.fedoraproject.org/ci/shared-tests"
git_ref = "main"
Comment on lines +1518 to +1519
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The git_repo and git_ref values are hardcoded strings. It would be more maintainable to define these as constants, especially if they might be reused or changed in the future. This makes the code easier to update and reduces the chance of errors if the values need to be modified.

# The shared plans will define their own provision steps, requiring
# compose to be turned off.
payload = self._get_tf_base_payload(distro, None)
payload["test"] = {
"tmt": {
"url": git_repo,
"ref": git_ref,
},
}
return payload
Comment on lines +1517 to +1529
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The newly added method _payload_shared is not trivial and should include a docstring explaining its purpose, arguments, and return value. This should follow the Google-style as specified in the repository's style guide. This improves code readability and maintainability.

Repository Style Guide: lines 378, 410

References
  1. Non-trivial methods of models must include docstrings describing the operation and accurate type hints, especially for the returned value, as incorrect type specified can lead to complicated run time issues. (link)
  2. With exception of trivial cases, all code must contain accurate and sufficiently detailed docstrings, formatted with accordance with the PEP 257 standard and in Google-style. (link)


@staticmethod
def is_fmf_configured(project: GitProject, metadata: EventData) -> bool:
try:
Expand Down
Loading