Skip to content

Conversation

@codeMaestro78
Copy link

CNCFInsertKubestronautInPeoople_json.py

Fixed two bugs:

  1. Logic flaw: The loop broke on first iteration regardless of duplicate match, allowing duplicates to be inserted if they weren't first in the list.

  2. Missing f-string prefix: '{newPeople.name}' was not being interpolated.

The duplicate check now iterates through ALL existing entries before inserting.

  • What was happening before:
for people in data:
    if people["name"].lower() == newPeople.name.lower():
        exit(2)  # Only caught duplicate if it was the FIRST entry
    else:
        data.insert(0, ...)  # Inserted and broke immediately!
        break
  • What happens now:
# Check ALL entries first
for people in data:
    if people["name"].lower() == newPeople.name.lower():
        print(f"{newPeople.name} already in people.json, abort!")
        exit(2)

# Only insert after confirming no duplicates
data.insert(0, ...)

#Bug

Logic flaw

  • Before : Loop checked first person, then immediately broke (either adding or exiting)
  • After : Loop iterates through ALL entries first to check for duplicates, only then inserts

f-string bug

  • Before : print("{newPeople.name} already...") (literal string)
  • after : print(f"{newPeople.name} already...") (interpolated)

Copilot AI review requested due to automatic review settings December 24, 2025 08:07
@github-actions github-actions bot added the needs-triage Indicates an issue or PR that has not been triaged yet (has a 'triage/foo' label applied) label Dec 24, 2025
@github-actions github-actions bot added needs-kind Indicates an issue or PR that is missing an issue type or kind (a kind/foo label) help wanted labels Dec 24, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes two bugs in the duplicate detection logic for inserting Kubestronauts into people.json: (1) a logic flaw where the loop would break on the first iteration regardless of whether a duplicate was found, and (2) a missing f-string prefix that prevented name interpolation in the error message.

Key Changes:

  • Separated duplicate checking from insertion logic - the loop now completes checking all entries before attempting to insert
  • Added f-string prefix to the error message for proper name interpolation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ple_json.py

Fixed three issues:

1. Logic flaw: The loop broke on first iteration regardless of duplicate match, allowing duplicates to be inserted if they weren't first in the list.

2. Missing f-string prefix: '{newPeople.name}' was not being interpolated.

3. Inconsistent state on duplicate: exit(2) would terminate before saving, leaving images renamed and ACKs done but JSON unsaved. Now skips duplicates gracefully with continue.

Signed-off-by: LALANI DEVARSHI <[email protected]>
@codeMaestro78 codeMaestro78 force-pushed the fix/kubestronaut-duplicate-detection-logic branch from 79c1cd2 to b0cdb96 Compare December 24, 2025 08:13
@codeMaestro78
Copy link
Author

Hi, @koksay
Can you please review this pr.

@koksay
Copy link
Member

koksay commented Dec 24, 2025

Hi @codeMaestro78 , sorry but I'm not the owner of this code. I believe CNCF folks are on vacation, you may not hear back until the new year.

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

Labels

help wanted needs-kind Indicates an issue or PR that is missing an issue type or kind (a kind/foo label) needs-triage Indicates an issue or PR that has not been triaged yet (has a 'triage/foo' label applied)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants