Leaseweb / terraform-provider-leaseweb

The Leaseweb terraform provider plugin
Apache License 2.0
9 stars 4 forks source link

feat: add server attributes in leaseweb_dedicated_server #41

Closed majidkarimizadeh closed 2 months ago

alrayyes commented 2 months ago

I know, work in progress. it's best practice to use imperative tense verbs. Ie: add server attributes to .... instead of added server attributes to....

imtiazPabel commented 2 months ago

Along with package name, I think we should also follow the standard about the methods name as well. Like dedicatedServer.all() / dedicatedServer.new() instead of dedicatedServer.dedicatedServerAll()/dedicatedServer.dedicatedServerNew()

https://go.dev/blog/package-names https://go.dev/doc/effective_go#package-names

alrayyes commented 2 months ago

From what I can tell the naming convention is using _ for private functions.

On Thursday, August 22nd, 2024 at 9:30 AM, Majid Karimizadeh @.***> wrote:

@majidkarimizadeh commented on this pull request.


In internal/facades/dedicated_server/data_adapters/to_data_source_model/adapter_test.go:

  • assert.Equal(t, int32(2), got.Specs.Ram.Size.ValueInt32())
  • assert.Equal(t, int32(1), got.Specs.Cpu.Quantity.ValueInt32())
  • assert.Equal(t, "id1", got.Specs.Hdds[0].Id.ValueString())
  • assert.Equal(t, "d", got.Specs.PciCards[0].Description.ValueString())
  • assert.Equal(t, "chassis", got.Specs.Chassis.ValueString())
  • assert.Equal(t, true, got.Specs.HardwareRaidCapable.ValueBool())
  • assert.Equal(t, "cid", got.Contract.Id.ValueString())
  • assert.Equal(t, "rid", got.Rack.Id.ValueString())
  • assert.False(t, got.FeatureAvailability.IpmiReboot.ValueBool())
  • assert.Equal(t, "rack", got.Location.Rack.ValueString())
  • assert.Equal(t, "name1", got.PowerPorts[0].Name.ValueString())
  • assert.Equal(t, "pid", got.PrivateNetworks[0].Id.ValueString())
  • assert.Equal(t, "public", got.NetworkInterfaces.Public.Mac.ValueString()) +}
  • +func Test_adaptDedicatedServers(t *testing.T) {

I think we should get rid _ in the tests. https://go.dev/doc/tutorial/add-a-test https://ieftimov.com/posts/testing-in-go-naming-conventions/

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because your review was requested.Message ID: @.***>

majidkarimizadeh commented 2 months ago

@alrayyes will be fixed in upcoming PR