Skip to content

Various QA fixes#13

Open
vincentdephily wants to merge 5 commits intocert-manager:mainfrom
vincentdephily:qa_unified_diff
Open

Various QA fixes#13
vincentdephily wants to merge 5 commits intocert-manager:mainfrom
vincentdephily:qa_unified_diff

Conversation

@vincentdephily
Copy link
Copy Markdown

@vincentdephily vincentdephily commented Feb 13, 2026

@inteon These have been extracted from #8, should be quick to review and interesting in their own right, and must be merged first.

@cert-manager-prow cert-manager-prow Bot added the dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. label Feb 13, 2026
@cert-manager-prow
Copy link
Copy Markdown
Contributor

cert-manager-prow Bot commented Feb 13, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jakexks for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cert-manager-prow cert-manager-prow Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 13, 2026
@cert-manager-prow
Copy link
Copy Markdown
Contributor

cert-manager-prow Bot commented Feb 13, 2026

Hi @vincentdephily. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@cert-manager-prow cert-manager-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 13, 2026
inteon
inteon reviewed Feb 13, 2026
View reviewed changes
Comment thread main.go
@cert-manager-prow cert-manager-prow Bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Feb 13, 2026
inteon
inteon reviewed Feb 13, 2026
View reviewed changes
Comment thread main.go
targetBase := flag.Arg(0)

dir, err := isDir(targetBase)
targets, err := getTargets(targetBase, templates, skippedDirs, verboseLogger)
Copy link
Copy Markdown
Member

@inteon inteon Feb 13, 2026

Choose a reason for hiding this comment

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

does this still work with single file targets?

Copy link
Copy Markdown
Author

@vincentdephily vincentdephily Feb 13, 2026

Choose a reason for hiding this comment

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

I'm pretty sure I had tested single-file at the time, but now I'm getting an error from the "git repository matcher" merged recently. I'll look for a fix.

Copy link
Copy Markdown
Author

@vincentdephily vincentdephily Feb 13, 2026

Choose a reason for hiding this comment

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

My bad, good catch.

I've restored the ability to scan single target, using a check at the start of getTargets() rather than a revert because it should simplify the later changes and because I'd like to keep main() simple.

@vincentdephily
qa: simplify code
83546a8 
There is no need to special-case handling a single file.

Signed-off-by: Vincent de Phily <vdephily@redhat.com>
@vincentdephily
qa: Delay reading file contents
1b428b6 
This reduces memory use by not reading all files upfront. It should
parallelize better if we later want to do that. It's shorter and IMHO
cleaner.

Signed-off-by: Vincent de Phily <vdephily@redhat.com>
@vincentdephily
fix: Reenable passing a single file argument
f627035 
Signed-off-by: Vincent de Phily <vdephily@redhat.com>
@vincentdephily
qa: Simplify skiplist handling, tweak verbose output
98f0137 
Signed-off-by: Vincent de Phily <vdephily@redhat.com>
@vincentdephily
qa: Use plain string instead of regex where appropriate
a1ed959 
Signed-off-by: Vincent de Phily <vdephily@redhat.com>
@vincentdephily vincentdephily mentioned this pull request Feb 13, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@inteon inteon inteon left review comments

Assignees

No one assigned

Labels

dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vincentdephily @inteon