Skip to content

Conversation

@sg-rh
Copy link
Collaborator

@sg-rh sg-rh commented Nov 12, 2025

No description provided.

@sg-rh sg-rh requested review from amp-rh and oharan2 November 12, 2025 22:17
@dfrazzette dfrazzette merged commit 657d4b3 into RedHatQE:main Nov 12, 2025
4 checks passed

steps = []

if self.is_rehearsal:
Copy link

Choose a reason for hiding this comment

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

I've seen several pattern are repeated, branching and settings. This is maintenance burden.

A much better approach is to branch 1x and store the deviating data once.

Say at line 49:

        if self.is_rehearsal:
            try:
                self.pr_id = pr_id or os.getenv("PULL_NUMBER") or self.name.split("-")[1]  # type: ignore
            except IndexError:
                self.logger.warning(
                    f"Pull number for job {self.name} not obtained, reporting may not be complete.",
                )
                self.pr_id = "1"
            self.logger.info(f"PR ID: {self.pr_id}")
            self.blobPfx = "pr-logs/pull/openshift_release/{self.pr_id}"
        else:
            self.pr_id = ""
            self.blobPfx = "logs"

Then we can safe us a lot of branches throughout the code and if any change on the path is only 1 place modification.

Another observation, not really related to this PR, why a lot of function parameters, such as job_name is marked with Optional[...] hint, while it's clearly inside the function code it can' handle None type? This is not good practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants