are these 73 lines of code really easier than just having the 1 sed 🤔 ?

It's by no means easier than sed, but it's more robust IMO. We can also modify this program later to avoid using dot imports by qualifying local types with the package name.

We can also modify this program later to avoid using dot imports by qualifying local types with the package name.

do we need to do that though 🤷 ?

It's by no means easier than sed, but it's more robust IMO.

I would disagree, the contents of addImport is pretty obscure, on a first pass I struggle to understand what its disagree. The sed is just a bit simpler to read, and much easier to test manually

it's more robust IMO

I'm trying to understand the pros/cons of each method, sed vs addDotImport. @cPu1 could you expand a bit on this, please? addDotImport adds quite a lot of complexity compared to sed. What do you see as the issues or limitations of sed, and how does addDotImport solve this and perform better?

If we do go with this program, I would suggest adding at least the minimum amount of unit tests to ensure that it really is more robust, that it does what we expect it to do, and to make it more maintainable.

@cPu1

@nikimanoledaki sed is not the right tool to manipulate Go sources. That sed snippet can generate programs that have syntax errors.

Here's what's wrong with using that sed snippet:

$ cat <<EOF | sed "/\"context\".*/a . \"github.com/aws/aws-sdk-go-v2/service/eks\""
// Code generated by ifacemaker; DO NOT EDIT.

package awsapi

import "context"

// CloudFormation provides an interface to the AWS CloudFormation service.
type CloudFormation interface {
 // ...
}

EOF

// Code generated by ifacemaker; DO NOT EDIT.

package awsapi

import "context"
. "github.com/aws/aws-sdk-go-v2/service/eks"

// CloudFormation provides an interface to the AWS CloudFormation service.
type CloudFormation interface {
 // ...
}


$ cat <<EOF | sed "/\"context\".*/a . \"github.com/aws/aws-sdk-go-v2/service/eks\""
// Code generated by ifacemaker; DO NOT EDIT.
// Copyright 2020 Weaveworks.
// The awsapi package provides an interface to AWS services. The "context" package is used to carry deadlines and
// cancellation signals.

package awsapi

import (
    "context"
)

// CloudFormation provides an interface to the AWS CloudFormation service.
type CloudFormation interface {
 // ...
}

EOF

// Code generated by ifacemaker; DO NOT EDIT.
// Copyright 2020 Weaveworks.
// The awsapi package provides an interface to AWS services. The "context" package is used to carry deadlines and
. "github.com/aws/aws-sdk-go-v2/service/eks"
// cancellation signals.

package awsapi

import (
    "context"
. "github.com/aws/aws-sdk-go-v2/service/eks"
)

// CloudFormation provides an interface to the AWS CloudFormation service.
type CloudFormation interface {
 // ...
}


$ cat <<EOF | sed "/\"context\".*/a . \"github.com/aws/aws-sdk-go-v2/service/eks\""
package awsapi

import (
    "fmt"
)

// CloudFormation provides an interface to the AWS CloudFormation service.
type CloudFormation interface {
 // ...
}

EOF

package awsapi

import (
    "fmt"
)

// CloudFormation provides an interface to the AWS CloudFormation service.
type CloudFormation interface {
 // ...
}

As for the maintenance of the add_import.go program, we have an issue for that: #4994

this can be removed right?

It's required. ServicesV2 is an embedded field of ProviderServices.

can we just put everything directly on the struct? it will be easier.

Also all the other PRs will just replace the existing apis with the new ones, STS is one of the few where it will live alongside the V1

I created a new struct because the service methods (STS() etc.) are implemented differently in that the service client is configured in the method itself. Additionally, there's a lot more happening in pkg/eks/api.go, so I wanted this to be cleaner and keep the diff minimal. I wouldn't mind adding it to the existing struct though, but this PR has already grown too much.

I can see both sides 😄

In reality, we don't need to worry too much about optimizing this area and creating all the services would be fine.

On the flip side, its a standard lazy initialization pattern. The one thing i would say is that we are assuming that all new services will use this pattern and that contributors will need to follow it. So, it might be worth documenting it somewhere?