azurerm_image - change os_disk property to a list #1443
Conversation
|
tests pass aside from failures due to too many virtual networks. |
azurerm/helpers/validate/url.go
Outdated
| "net/url" | ||
|
|
||
| "github.com/hashicorp/terraform/helper/schema" | ||
| "strings" |
There was a problem hiding this comment.
can you move this up to the first group of imports?
azurerm/helpers/validate/url.go
Outdated
| "strings" | ||
| ) | ||
|
|
||
| func UrlIsHttpOrHttps(i interface{}, k string) (_ []string, errors []error) { |
There was a problem hiding this comment.
There may have been a precedent here you are following, but traditionally initialisms in Go are capitalized: https://github.com/golang/go/wiki/CodeReviewComments#initialisms
URLIsHTTPOrHTTPS I think? I may be misreading this thing from Go, may want to run the linter on it.
azurerm/helpers/validate/url.go
Outdated
| return UrlWithScheme([]string{"http", "https"})(i, k) | ||
| } | ||
|
|
||
| func UrlWithScheme(validSchemes []string) schema.SchemaValidateFunc { |
There was a problem hiding this comment.
See other comment on initialisms.
azurerm/helpers/validate/url_test.go
Outdated
| "testing" | ||
| ) | ||
|
|
||
| func TestHelper_Validate_UrlIsHttpOrHttps(t *testing.T) { |
There was a problem hiding this comment.
I would just call this Test<function name> and if its one of many methods Test<function name>_<description>
azurerm/helpers/validate/url_test.go
Outdated
| } | ||
|
|
||
| for _, tc := range cases { | ||
| _, errors := UrlIsHttpOrHttps(tc.Url, "test") |
There was a problem hiding this comment.
these cases should be inside a t.Run
|
Tests have been switched over to |
katbyte
force-pushed
the
b-image-osdisk_as_list
branch
2 times, most recently
from
c2fa751 to
2b20c17
Compare
katbyte
force-pushed
the
b-image-osdisk_as_list
branch
from
2b20c17 to
a9d730e
Compare
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
Should fix #234
Also adds some validation