Skip to content

Consider not requiring SVN credentials for dry-run#164

Merged
iamdharmesh merged 3 commits into
10up:developfrom
iftakharul-islam:develop
Feb 10, 2025
Merged

Consider not requiring SVN credentials for dry-run#164
iamdharmesh merged 3 commits into
10up:developfrom
iftakharul-islam:develop

Conversation

@iftakharul-islam
Copy link
Copy Markdown
Contributor

@iftakharul-islam iftakharul-islam commented Feb 7, 2025

SVN credentials are only required when the script is performing a real deployment (not during dry-run).

Closes #149

How to test the Change

  • Set dry-run: true.
  • Run the script with missing SVN credentials (SVN_USERNAME and SVN_PASSWORD).
  • Ensure the script does not exit and displays a warning message indicating that the credentials are missing but continues the dry-run.

Credits

Props @aaemnnosttv ...

Checklist:

@iftakharul-islam iftakharul-islam requested a review from a team as a code owner February 7, 2025 06:01
@iftakharul-islam iftakharul-islam requested review from Sidsector9 and removed request for a team February 7, 2025 06:01
@jeffpaul jeffpaul added this to the 2.4.0 milestone Feb 7, 2025
@jeffpaul jeffpaul requested review from iamdharmesh and removed request for Sidsector9 February 7, 2025 18:08
Copy link
Copy Markdown
Member

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

Hi @iftakharul-islam 👋, Thanks for the PR, it looks great! I just wanted to share a minor suggestion: how about adding a warning in dry-run in case SVN credentials are missing to make developers aware of it, as suggested in the issue description? Thanks a lot for your help with this!

@iftakharul-islam
Copy link
Copy Markdown
Contributor Author

@iamdharmesh Thanks for your feedback and I've added SVN secrets warning.

Copy link
Copy Markdown
Member

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @iftakharul-islam. LGTM.

@iamdharmesh iamdharmesh merged commit 5b325a1 into 10up:develop Feb 10, 2025
@dkotter dkotter modified the milestones: 2.4.0, 2.3.1 Feb 10, 2025
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.

Consider not requiring SVN credentials for dry-run

4 participants