Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(gce): Refactor BasicGoogleDeployHandler to be more readable and testable #6290

Merged
merged 15 commits into from
Nov 20, 2024
Merged

refactor(gce): Refactor BasicGoogleDeployHandler to be more readable and testable #6290

merged 15 commits into from
Nov 20, 2024

Conversation

edgarulg
Copy link
Contributor

@edgarulg edgarulg commented Oct 10, 2024 •
edited
Loading

Key point about the refactor.

  1. I translated the whole class BasicGoogleDeployHandler into java.
  2. I split up the handle method into small single responsibility methods.
  3. I defined unit testing for most of the new methods I created to prove functionality.
  4. I do not include any new feature or change that was not defined by the BasicGoogleDeployHandler before.

Related to: spinnaker/spinnaker#6985

@edgarulg edgarulg mentioned this pull request Oct 10, 2024
Open
@edgarulg
Copy link
Contributor Author

I published and deployed a snapshot version with my refactor and I validated a GCE deploy worked as expected. I didn't tests all possible configurations as part of my e2e validation.

@edgarulg
Copy link
Contributor Author

@Mergifyio refresh

@mergify Mergify
Copy link
Contributor

mergify bot commented Oct 23, 2024

refresh

✅ Pull request refreshed

@edgarulg
Copy link
Contributor Author

@Mergifyio rebase

@mergify Mergify
Copy link
Contributor

mergify bot commented Oct 23, 2024

rebase

☑️ Nothing to do

  • -conflict [📌 rebase requirement]
  • -closed [📌 rebase requirement]
  • queue-position = -1 [📌 rebase requirement]
  • any of:
    • #commits > 1 [📌 rebase requirement]
    • #commits-behind > 0 [📌 rebase requirement]
    • -linear-history [📌 rebase requirement]

@jasonmcintosh jasonmcintosh added the ready to merge Approved and ready for a merge label Nov 20, 2024
@mergify mergify bot added the auto merged Merged automatically by a bot label Nov 20, 2024
@mergify mergify bot merged commit bbcd4c7 into spinnaker:master Nov 20, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbyron-sf dbyron-sf dbyron-sf left review comments

@jasonmcintosh jasonmcintosh jasonmcintosh left review comments

@mattgogerly mattgogerly mattgogerly approved these changes

@plumpy plumpy Awaiting requested review from plumpy plumpy is a code owner

@skandragon skandragon Awaiting requested review from skandragon skandragon is a code owner

@link108 link108 Awaiting requested review from link108

@christosarvanitis christosarvanitis Awaiting requested review from christosarvanitis

Assignees
No one assigned
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for a merge target-release/1.37
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@edgarulg @jasonmcintosh @mattgogerly @dbyron-sf @spinnakerbot