First version for the provider, supports templates and variables

[shlomimatichin]

includes a test suite, plz take a look at the end of README.md for details

[roni-frantchi]

Hey @shlomimatichin
So code looks good to me, albeit admittedly I could not thoroughly review this one as it is very hard to review 3K lines of code in one PR - let's not do this again.

So having said that - some general thoughts and comments on structure and tests:

1. Looking at how other provider projects are structured (specifically ones with api in the same project), I would have two simply named directories at the root of the repo: api and provider. Each should have its own modules and unit tests. Under the provider dir, it seems like all providers common have a directory with the provider name (namely env0) where the provider code will reside.
2. For different templates (and integration tests) - they seem to have an examples directory
3. All providers seem to have unit tests - such as https://github.com/hashicorp/terraform-provider-kubernetes/blob/master/kubernetes/data_source_kubernetes_config_map_test.go

I'm just looking at various 1st and 3rd party providers and see what the common standard is - I think our provider should try and match those.

[shlomimatichin]

@roni-frantchi
0. regarding size of PR - this was by request from you guys. if you guys request yet another ~3k lines of code, that's how many lines need to be reviewed... i can offer to split this up artificially, or maybe an online session for some guided walkthrough of the code.

1. i took insparation from the project omri referenced me to, your references seem to differ. i understood the logic was not to write a provider the way hashicorp writes them, but the way the community writes them, since that makes more sense in this context. also, regarding naming, i do prefer env0apiclient over api. for me that helps when reading the code, and also, that subdirectory can be imported to other go project in the future, as it's not terraform specific. also, if it's not a red line for you, i also prefer env0tfprovider over provider. for me that additional prefix helps to avoid confusion with other possible libraries.
2. i'll move the test cases from testing to examples, ack on this making the example easier to find.
3. imho, test in attached link is redundantly repeating the code, adds no additional confidence or coverage. (unlike the full tests currently implemented) and the other unit tests i observed have a similar "unittest for the sake of unittest" feel to them. they cover terraform functions like base64encode and correct hcl->data assignments. our case is even simpler as the assigned data is only strings (or list of strings), so you don't even need to think of "dictionary objects". assuming i haven't convinced you, and you are still interested in unit-tests in the project, may i ask to receive a more specific request for what you wish to cover in unit-test? i don't think it's too much work to merit a philosophical showdown, afaiac we can agree to disagree.
4. yes, my methodology was to copy the code, structure, and logic from the references i was given, so ack on the general idea, and sorry that doesn't show.

[roni-frantchi]

this was by request from you guys

We don't dictate PR size nor content - I think splitting it up to a PR adding one general structure (like the one I had comments on), then a PR for api client, then a PR for the provider, would make perfect sense in making those very different aspects easier to review and is by no means "artificial".

write a provider the way hashicorp writes them, but the way the community writes them

From the examples I've gathered those are fairly similar. (see the example on the provided link, which isn't by Hashi)

for me that additional prefix helps to avoid confusion with other possible libraries.

I understand. But it does deviate from every other provider I have encountered. So please make the change.

unittest for the sake of unittest

To me those help to specify the responsibility of each component and its behavior - they help assert the chosen structure makes sense as well.
It's documentation of behavior by code and it makes it easier to maintain.
And finally it is industry standard and seem to be that way with every provider we encounter - so let us match the community's expectation.

yes, my methodology was to copy the code, structure, and logic from the references i was given, so ack on the general idea, and sorry that doesn't show.

It could very well be that a bad reference was given - covering 3-4 of those helps comprehend what's common practice and should show here

[shlomimatichin]

closing this, going to split to multiple smaller pull requests.

[avnerenv0]

Due to lack of bandwith in the team, we'll try to push this first and make the improvements later

[avnerenv0]

I'm gonna merge this as is, and from there I'll open other issues/PR's