Beta

[xlr-8]

No description provided.

[ifraixedes]

I will make a type of batch errors like this

type Errs []Err

func (errs Errs) Error() string {
  	var output [][]string
  
  	for _, e := range errs {
  		output = append(output, []string{e.Region(), e.Service()})
  	}
  	return fmt.Sprintf("%d error(s) occured: %s", len(errs), output)
  }

Then we don't need the AppendError we could just do

var errs Errs

errs = append(errs, NewAPIError(e, "the region", "the service"))

Having this, we can return Errs as nil without having to handle if the slice have values or not

[xlr-8]

Indeed, good idea - will apply

[ifraixedes]

Because of https://github.com/cycloidio/raws/pull/1/files#r122167049

I would return and Errs type rather than an error interface, it isn't a problem for the caller because Errs satisfies the error interface

This comment applies to all the functions that the package has now and they return RawsErr type as error interface

[xlr-8]

But as Errs satisfies error, why returning it, rather than an error type? As we discussed, my understanding was that it was more standard, and would simply make more sense. No?

[ifraixedes]

Ups, I didn't want to mean that.

For such reason, I was complaining about AWS SDK returns a generic error than their specific error interfaces on methods that they can only return one specific error type. If their function can return multiple interface type of errors in a function, you have to choose the narrowest interface which is satisfied by all of those, obviously the error interface should be satisfied by all of them, so there is always that chance.

I would like to remember the right terminology but if I don't remember bad (I'm shit remember names and terminology), a function which is more extensible must be covariance in their arguments and contravariance in their return values (Covariance and contravariance (computer science)

What's standard is to return an value which satisfies the error interface. What I wanted to mean is that returning type struct rather than a type interface has more limitations for the callers and from the creator point of view, could lead to export some struct fields that they can be removed later on, but the callers are relying on them, so it's more prone to break compatibility than using interfaces.

For this reason, I suggest to return the interface.

[xlr-8]

I believe I took all notes into considerations!

[ifraixedes]

Because it's used reflect.DeepEqual the parameters wouldn't have needed to be changed and the function will be more generic for any test. Being a test we don't need to show any semantics in docs which types can provide

[xlr-8]

I simply thought, that as we were using our own type, it would make more sense to use it there as well. But that would indeed work with both, I'll revert it.

[ifraixedes]

What's the reason behind this change? (With this change I mean the change in the type which is declared the methods required by the interface)

[xlr-8]

Hum, it's a mistake from my side, as when using Errs I couldn't have a pointer receiver for Error() otherwise I would hit: Errs does not implement error (Error method has pointer receiver); and I wrongfully applied it to callErr as well.

[coveralls]

Changes Unknown when pulling 0cdf330 on beta into ** on master**.

[xescugc]

Overall I see no errors, just some punctuations (it's a big PR hehe)

For the documentation I recomend you to read this post which explains the go way.

Notice this comment is a complete sentence that begins with the name of the element it describes. This important convention allows us to generate documentation in a variety of formats, from plain text to HTML to UNIX man pages, and makes it read better when tools truncate it for brevity, such as when they extract the first line or sentence.

For the code everithing is clear (I read it in an overview as its big) but I think that it's well formatted, ther are some funcions in which you could use a go syntax suggar on declare repeated types on the arguments, example you could do:

   func NewConnector(accessKey, secretKey string, regions []string, config *aws.Config) (*Connector, error) { ... }

instead of

  func NewConnector(accessKey string, secretKey string, regions []string, config *aws.Config) (*Connector, error) {

Other things are opiniated, the use of var instead of := everyone has an opinion on that :)

[xlr-8]

Thank you for the comments, I'll take a look at the doc link.

Regarding the notation, for me it really depends of the situation.
When the variables are just declared, and used later on, then it makes sense to me to do so, I find it also more explicit when using those in an example. However for directly used/initialized variable within the code, then I don't see the problem with that, as it's self-explicit.

And on the arguments, I personally prefer to use the shortened version, only when all arguments have the same type, I find it confusing otherwise.
But as you said, everyone has his opinion :)

[ifraixedes]

I also prefer to use the long syntax to declare variables var ... and only to use the short syntax for loops (for i := ...) and conditionals (if err := ) because

1. Go doesn't compile if you use var on loops on conditionals e.g. for var i = ...
2. Even though it would compile, I'm not sure if having the var word after for and if could be quite unreadable.

Obviously 1 enforce to use the short syntax on such cases if you only want to declare the variable inside of the loop and conditional scope

Regarding the Go syntax sugar for the types of the function arguments, I wold have prefer that it won't exist, so I prefer to use the long syntax, rather than the short, although it's something that, at the beginning I used the short syntax, later I've been doubting between both and now I prefer the long one.

The case that @xlr-8 mentions could be a good case to consider to use it, however I'm still not sold on using both rather than one.

As you commented this is a personal opinion, so I don't think that we should enforce one, they are minor things to remember and because Go has a short set of instructions and statements and syntax sugar tricks, every Go developer know them, so the syntax looks familiar. If you would like to enforce one, I'm not agains but only if we can detect them through a linter.