Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 25, 2025

Changes proposed in this pull request

  • Extract timestamp directly from backup filename (yyyyMMddHHmmss prefix) for ordering instead of relying on File.GetCreationTime
  • Add ExtractTimestampFromFilename method to TimestampHelper.cs in Utilities project to parse filename timestamps with InvariantCulture for consistent behavior
  • Update CleanupBackups to use extracted timestamps, ensuring reliable ordering across file systems and copy operations
  • Refactor ExtractTimestampFromFilename as a public utility method in TimestampHelper.cs, making it reusable across the codebase

Related issue(s)

Copilot generated summary

Copilot summary

File creation timestamps are unreliable across different file systems and after file operations like copying. Since backup filenames follow the format yyyyMMddHHmmss_Settings.json (e.g., 20241225182301_Settings.json), we now extract and parse this embedded timestamp for ordering. Invalid filenames return DateTime.MinValue to sort as oldest.

Changes:

  • Added ExtractTimestampFromFilename helper method to TimestampHelper.cs in Utilities with proper culture-invariant parsing
  • Made the method public and general-purpose for reuse across the codebase
  • Modified CleanupBackups ordering from File.GetCreationTime(f) to TimestampHelper.ExtractTimestampFromFilename(f)
  • Updated documentation to reflect new ordering approach
  • Removed logging dependency from utility method (Utilities shouldn't depend on log4net)

To-Do

Contributing

By submitting this pull request, I confirm the following:


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Update daily settings backup for filename date extraction Use filename timestamp for backup ordering instead of file creation time Dec 25, 2025
Copilot AI requested a review from BornToBeRoot December 25, 2025 18:30
@BornToBeRoot
Copy link
Owner

@copilot move the method ExtractTimestampFromFilename to the Utilities (in an existing file or create a new if necessary)

Copy link
Contributor Author

Copilot AI commented Dec 25, 2025

@copilot move the method ExtractTimestampFromFilename to the Utilities (in an existing file or create a new if necessary)

Done! Moved ExtractTimestampFromFilename to TimestampHelper.cs in the Utilities project. The method is now public and more general-purpose, and I removed the logging dependency since Utilities shouldn't depend on log4net. See commits f56dc1c and e78a716.

Copilot AI changed the title Use filename timestamp for backup ordering instead of file creation time Use filename timestamp for backup ordering and move to Utilities Dec 25, 2025
@BornToBeRoot BornToBeRoot requested a review from Copilot December 25, 2025 21:17
@BornToBeRoot BornToBeRoot marked this pull request as ready for review December 25, 2025 21:17
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 refactors the backup file ordering mechanism to use timestamps extracted directly from filenames instead of relying on file system creation times, ensuring consistent behavior across different file systems and after file operations.

  • Extracts timestamps from backup filenames (yyyyMMddHHmmss prefix) for reliable ordering
  • Adds three new utility methods to TimestampHelper for timestamp-based filename operations
  • Updates CleanupBackups to use filename-based timestamps with culture-invariant parsing

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File Description
Source/NETworkManager.Utilities/TimestampHelper.cs Adds three new public methods: GetTimestampFilename, IsTimestampedFilename, and ExtractTimestampFromFilename for generating, validating, and parsing timestamped filenames
Source/NETworkManager.Settings/SettingsManager.cs Refactors backup creation calls to use GetTimestampFilename and updates CleanupBackups to filter and order backups using filename-based timestamps instead of file creation time

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

@BornToBeRoot BornToBeRoot merged commit 35e88cc into feature/daily_settings_backup Dec 25, 2025
1 of 4 checks passed
@BornToBeRoot BornToBeRoot deleted the copilot/sub-pr-3283 branch December 25, 2025 22:21
@BornToBeRoot BornToBeRoot added this to the next-release milestone Dec 25, 2025
mergify bot pushed a commit that referenced this pull request Dec 25, 2025
* Feature: Create a daily settings backup

* Chore: Cleanup

* Docs: #3283

* Chore: Cleanup

* Fix: Fixes based on copilot feedback

* Use filename timestamp for backup ordering and move to Utilities (#3284)

* Initial plan

* Extract date from filename for backup ordering

Co-authored-by: BornToBeRoot <[email protected]>

* Use InvariantCulture and specific exception handling

Co-authored-by: BornToBeRoot <[email protected]>

* Move ExtractTimestampFromFilename to TimestampHelper in Utilities

Co-authored-by: BornToBeRoot <[email protected]>

* Update comment to match specific exception handling

Co-authored-by: BornToBeRoot <[email protected]>

* Feature: Improve check

* Update Source/NETworkManager.Utilities/TimestampHelper.cs

Co-authored-by: Copilot <[email protected]>

* Update Source/NETworkManager.Utilities/TimestampHelper.cs

Co-authored-by: Copilot <[email protected]>

* Update Source/NETworkManager.Utilities/TimestampHelper.cs

Co-authored-by: Copilot <[email protected]>

* Update TimestampHelper.cs

* Update TimestampHelper.cs

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: BornToBeRoot <[email protected]>
Co-authored-by: Copilot <[email protected]>

* Feature: Some logging / docs

---------

Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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