Skip to content

feat: add --diff-yes flag to copilot [noun] deploy commands#4832

Merged
mergify[bot] merged 5 commits intoaws:mainlinefrom
efekarakus:feat/issues-4825
May 3, 2023
Merged

feat: add --diff-yes flag to copilot [noun] deploy commands#4832
mergify[bot] merged 5 commits intoaws:mainlinefrom
efekarakus:feat/issues-4825

Conversation

@efekarakus
Copy link
Copy Markdown
Contributor

Resolves #4825

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

@efekarakus efekarakus requested a review from a team as a code owner May 3, 2023 20:35
@efekarakus efekarakus requested review from Lou1415926 and removed request for a team May 3, 2023 20:35
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 3, 2023

🍕 Here are the new binary sizes!

Name New size (kiB) size (kiB) Delta (%)
macOS (amd) 50620 50328 +0.58
macOS (arm) 50816 50516 +0.59
linux (amd) 44556 44304 +0.57
linux (arm) 42820 42564 +0.60
windows (amd) 41436 41204 +0.56

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 3, 2023

Codecov Report

Merging #4832 (5ccdefd) into mainline (441f218) will increase coverage by 0.00%.
The diff coverage is 77.77%.

❗ Current head 5ccdefd differs from pull request most recent head a187f77. Consider uploading reports for the commit a187f77 to get more accurate results

@@            Coverage Diff            @@
##           mainline    #4832   +/-   ##
=========================================
  Coverage     70.09%   70.10%           
=========================================
  Files           287      287           
  Lines         41233    41241    +8     
  Branches        280      280           
=========================================
+ Hits          28904    28910    +6     
- Misses        10939    10941    +2     
  Partials       1390     1390           
Impacted Files Coverage Δ
internal/pkg/cli/env_deploy.go 56.96% <75.00%> (+0.30%) ⬆️
internal/pkg/cli/svc_deploy.go 36.24% <80.00%> (+0.33%) ⬆️

Comment on lines +265 to +268
contd, err := o.skipDiffPrompt, nil
if !o.skipDiffPrompt {
contd, err = o.prompt.Confirm(continueDeploymentPrompt, "")
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's okay to write ⬇️ here? Because if we got --diff-yes that means noDeploy should render to false 🤔

if o.skipDiffPrompt {
 return true, nil
}

Copy link
Copy Markdown
Contributor Author

@efekarakus efekarakus May 3, 2023

Choose a reason for hiding this comment

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

oh I wasn't sure because l.277 in the same method seems to do the actual deployment, so if I were to return it'd skip the deployment altogether I think 💭

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh lol that's right

Comment thread internal/pkg/cli/svc_deploy_test.go Outdated
@mergify mergify Bot merged commit 57c5cdb into aws:mainline May 3, 2023
mergify Bot pushed a commit that referenced this pull request Jul 12, 2023
Related: #4832

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
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.

Adding option to pass --yes to copilot deploy --diff

4 participants