Skip to content

fix(cli): address PR review comments - cross-platform time, formatting, unused var#374

Merged
wax911 merged 2 commits into
build/ochestration-managementfrom
copilot/sub-pr-322
Jan 9, 2026
Merged

fix(cli): address PR review comments - cross-platform time, formatting, unused var#374
wax911 merged 2 commits into
build/ochestration-managementfrom
copilot/sub-pr-322

Conversation

Copy link
Copy Markdown

Copilot AI commented Jan 7, 2026

Addresses code review feedback on tools/stackctl_cli.py from PR #322: cross-platform compatibility issue, formatting nitpick, and unused variable.

Changes

  • Cross-platform compatibility: Replace os.times().elapsed with time.time() for backup timestamp generation (line 366)
  • Formatting: Remove trailing space in logs() function signature (line 258)
  • Code cleanup: Remove unused proc variable in secrets_decrypt() (line 403)

Context

The os.times().elapsed field is not available on all platforms and raises AttributeError on Windows. Using time.time() provides consistent cross-platform behavior for generating unique backup filenames.

# Before (platform-specific)
backup = envp.with_suffix(envp.suffix + f".bak.{int(os.times().elapsed)}")

# After (cross-platform)
backup = envp.with_suffix(envp.suffix + f".bak.{int(time.time())}")

The set +e/set -e pattern mentioned in review was already fixed in commit a0bd17d.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…g, unused var

Co-authored-by: wax911 <7859175+wax911@users.noreply.github.com>
Copilot AI changed the title [WIP] Add Python stackctl CLI with overlays and SOPS support fix(cli): address PR review comments - cross-platform time, formatting, unused var Jan 7, 2026
Copilot AI requested a review from wax911 January 7, 2026 15:00
@wax911 wax911 marked this pull request as ready for review January 9, 2026 13:24
@wax911 wax911 merged commit 1de7bd0 into build/ochestration-management Jan 9, 2026
3 checks passed
@wax911 wax911 deleted the copilot/sub-pr-322 branch January 9, 2026 13:26
wax911 added a commit that referenced this pull request Mar 12, 2026
…g, unused var (#374)

* Initial plan

* fix(cli): address PR review comments - cross-platform time, formatting, unused var

Co-authored-by: wax911 <7859175+wax911@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: wax911 <7859175+wax911@users.noreply.github.com>
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.

2 participants