Skip to content

Conversation

@AakashSuresh2003
Copy link
Contributor

@AakashSuresh2003 AakashSuresh2003 commented Dec 1, 2025

#1211)

  • Did you write/update appropriate tests
  • Release notes updated (if appropriate)
  • Appropriate logging output
  • Issue linked
  • Docs updated (or issue created)
  • New package licenses are added to ThirdPartyNotices.txt (if applicable)

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 adds path validation and logging for bbs2gh archive operations to provide better error messages to users when invalid paths are specified. The changes address issue #1211 by validating that archive paths and BBS shared home directories exist before attempting migration operations.

  • Adds validation for --archive-path to ensure the file exists when explicitly provided by users
  • Adds validation for --bbs-shared-home to ensure the directory exists when running on the Bitbucket instance (not using SSH/SMB download)
  • Adds logging to output the archive path before upload operations for better debugging
  • Introduces DirectoryExists method to FileSystemProvider to support directory validation

Reviewed changes

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

File Description
src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs Added path validation for archive files and BBS shared home directories in ValidateOptions, and added logging for archive path before upload operations
src/Octoshift/Services/FileSystemProvider.cs Added DirectoryExists method to support directory existence validation, following the same pattern as existing FileExists method
src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs Added comprehensive test coverage including tests for validation failures, tests ensuring validation is skipped when using SSH/SMB, and test verifying archive path logging

@github-actions
Copy link

github-actions bot commented Dec 1, 2025

Unit Test Results

  1 files    1 suites   10m 23s ⏱️
960 tests 960 ✅ 0 💤 0 ❌
961 runs  961 ✅ 0 💤 0 ❌

Results for commit 1390b46.

♻️ This comment has been updated with latest results.

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@AakashSuresh2003
Copy link
Contributor Author

Hi @brianaj,

Could you please review my PR and provide any suggestions?

@brianaj
Copy link
Collaborator

brianaj commented Dec 2, 2025

@AakashSuresh2003 were you able to functionally test it (screenshots would be nice)? And can you add release notes.

@AakashSuresh2003
Copy link
Contributor Author

Hi @brianaj,

The functional test screenshot is attached below, and the release notes have been added
Please let me know if there’s anything else needed from my side.

image

@github-actions
Copy link

github-actions bot commented Dec 2, 2025

Code Coverage

Package Line Rate Branch Rate Complexity Health
ado2gh 72% 70% 712
Octoshift 83% 72% 1737
bbs2gh 83% 78% 663
gei 81% 72% 574
Summary 81% (7734 / 9600) 73% (1837 / 2519) 3686

@brianaj brianaj self-requested a review December 2, 2025 19:20
@brianaj
Copy link
Collaborator

brianaj commented Dec 2, 2025

Thanks will work on shipping this in #1462 so I can run CI.

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