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.

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

chore: private services env compatibility checker, only enable if enabled 🤦 #4150

Closed
wants to merge 12 commits into from

Conversation

dannyrandall
Copy link
Contributor

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

@dannyrandall dannyrandall requested a review from a team as a code owner November 1, 2022 21:58
@dannyrandall dannyrandall requested review from KollaAdithya and removed request for a team November 1, 2022 21:58
@codecov-commenter
Copy link

codecov-commenter commented Nov 1, 2022

Codecov Report

Merging #4150 (df59371) into mainline (759ccbc) will increase coverage by 0.00%.
The diff coverage is 42.85%.

@@            Coverage Diff            @@
##           mainline    #4150   +/-   ##
=========================================
  Coverage     69.10%   69.11%           
=========================================
  Files           251      251           
  Lines         36037    36042    +5     
  Branches        264      264           
=========================================
+ Hits          24904    24910    +6     
+ Misses         9929     9927    -2     
- Partials       1204     1205    +1     
Impacted Files Coverage Δ
internal/pkg/manifest/svc.go 79.34% <0.00%> (ø)
internal/pkg/template/env.go 53.52% <ø> (ø)
internal/pkg/manifest/rd_web_svc.go 76.54% <40.00%> (-2.41%) ⬇️
...rnal/pkg/deploy/cloudformation/stack/rd_web_svc.go 92.85% <100.00%> (ø)
...ternal/pkg/deploy/cloudformation/cloudformation.go 72.47% <0.00%> (-0.55%) ⬇️
internal/pkg/manifest/workload.go 83.04% <0.00%> (+0.49%) ⬆️
internal/pkg/manifest/loader.go 74.19% <0.00%> (+12.90%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

huanjani
huanjani previously approved these changes Nov 1, 2022
@github-actions
Copy link

github-actions bot commented Nov 3, 2022

🍕 Here are the new binary sizes!

Name New size (kiB) v1.23.0 size (kiB) Delta (%)
macOS (amd) 47812 47548 +0.56
macOS (arm) 47812 48200 ❤️ -0.80
linux (amd) 42044 41812 +0.55
linux (arm) 42044 41220 🥺 +2.00
windows (amd) 38872 38664 +0.54

internal/pkg/manifest/loader_test.go Outdated Show resolved Hide resolved
internal/pkg/deploy/cloudformation/stack/rd_web_svc.go Outdated Show resolved Hide resolved
@@ -186,6 +191,9 @@ func (s RequestDrivenWebService) applyEnv(envName string) (workloadManifest, err

func (s *RequestDrivenWebService) requiredEnvironmentFeatures() []string {
Copy link
Contributor

Choose a reason for hiding this comment

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

This has nothing to do with your PR but it's pretty strange to me that this method:

func (s *DynamicWorkloadManifest) RequiredEnvironmentFeatures() []string {

belongs to *DynamicWorkloadManifest 🤔 it doesn't seem relevant to a "dynamic" aspect.
I'd have expected it to be part of the individual Manifest types.

internal/pkg/template/env.go Show resolved Hide resolved
internal/pkg/manifest/transform.go Outdated Show resolved Hide resolved
@dannyrandall dannyrandall marked this pull request as draft November 18, 2022 22:34
@dannyrandall dannyrandall marked this pull request as ready for review November 18, 2022 23:52
@iamhopaul123 iamhopaul123 added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Nov 28, 2022
@Lou1415926 Lou1415926 removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Nov 29, 2022
@Lou1415926 Lou1415926 added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jan 17, 2023
@Lou1415926 Lou1415926 removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jan 17, 2023
@iamhopaul123 iamhopaul123 added do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. and removed do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. labels Feb 21, 2023
@efekarakus efekarakus added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Mar 27, 2023
@dannyrandall dannyrandall marked this pull request as draft April 14, 2023 19:42
@bvtujo bvtujo removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Sep 1, 2023
@Lou1415926 Lou1415926 added do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. and removed do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. labels Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants