Skip to content

Update reload.go - #2334

Closed
augustbleeds wants to merge 1 commit into
mainfrom
augustus.update-reload-method
Closed

augustbleeds wants to merge 1 commit into
mainfrom
augustus.update-reload-method

Conversation

@augustbleeds

@augustbleeds augustbleeds commented Dec 18, 2025 •

Copy link
Copy Markdown

I'd like to use the reload method outside of tests. This was possible on prior versions of CTF.


Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.

Why

The changes refactor the UpgradeNodeSet function to introduce flexibility in specifying a custom tag for the unique identifier generation. This allows for more descriptive and potentially context-specific tagging of log files, aiding in better log management and identification.

What

  • framework/components/simple_node_set/reload.go
    • Modified UpgradeNodeSet to call a new function UpgradeNodeSetWithTag using t.Name() as the tag.
    • Added a new function UpgradeNodeSetWithTag that accepts a tag parameter for custom unique identifier generation, facilitating more descriptive log naming. This function encapsulates the original logic of UpgradeNodeSet, enhancing it with the ability to use custom tags.

@augustbleeds
augustbleeds requested a review from a team as a code owner December 18, 2025 20:55
Copilot AI review requested due to automatic review settings December 18, 2025 20:55
@github-actions

Copy link
Copy Markdown

👋 augustbleeds, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

Copilot AI left a comment

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.

Pull request overview

This PR refactors the UpgradeNodeSet function to enable its use outside of testing contexts. Previously, the function required a *testing.T parameter, limiting its usage to test files. The refactoring extracts the core logic into a new UpgradeNodeSetWithTag function that accepts a string tag instead, while maintaining backward compatibility through the original function.

Key Changes

  • Introduced UpgradeNodeSetWithTag function that accepts a string tag parameter instead of *testing.T
  • Refactored UpgradeNodeSet to delegate to UpgradeNodeSetWithTag using t.Name() as the tag

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

uniq := fmt.Sprintf("%s-%s-%s", framework.DefaultCTFLogsDir, t.Name(), uuid.NewString()[0:4])
return UpgradeNodeSetWithTag(t.Name(), in, bc, wait)
}

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

The new UpgradeNodeSetWithTag function lacks documentation. Add a comment explaining its purpose, parameters, and how it differs from UpgradeNodeSet, similar to the existing documentation on line 15-16.

Suggested change
// UpgradeNodeSetWithTag performs the same node set upgrade as UpgradeNodeSet but allows
// the caller to provide an explicit tag used when saving container logs. The tag is
// incorporated into the log directory name instead of relying on t.Name(), which can be
// useful when running outside of testing.T or when finer-grained tagging is desired.

Copilot uses AI. Check for mistakes.
@kalverra

Copy link
Copy Markdown
Contributor

@augustbleeds Closing this as stale

@kalverra kalverra closed this Mar 27, 2026
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