Closed taraspos closed 6 years ago
Good job @Trane9991 . It seems you quickly got a grasp of our new way of wrapping around AWS (i.e. cloud commands), although there are still some rough edges and things to improve on our side.
Thanks for the work so far! We do not have a section on how to contribute to awless, so I will give you random hints that can help (you might already know):
gen
folder (need a lot of cleaning actually in term of making the code more readable). You should not need to touch this part.gen_
prefix. They should not be edited obviously.go generate generate.go
when modifying cloud commands (as you did) or the models in general. This call should be idempotent.aws/doc/paramsdoc.go
(next to aws/doc/gen_paramsdoc.go)To validate the project we then have:
go test ./... -v
that you can do whenever./smoke_test/smoke_test.sh
and a mastondonte template but those ones cost money (create/revert stuff on the cloud) so you don't have to do it.More importantly, we now have acceptance tests for each cloud commands. In your case, this would be in the file aws/acceptance/stack_test.go
. Could you add the corresponding sub tests? They should be feasible.
Also I am wondering if the dep prune
should be done in another PR so this one could be merged without risking a unexpected effect, and we could revert commits independently of the other.
Hey,
added tests, descriptions, reverted dep prune
so we do it in separate PR, also added small Makefile to generate, test and build code.
Did a quick review. Thanks for the work.
Before merging if you don't mind it would be good to:
Negative tests is a very good idea. I tried it locally, but it would be nice to also add them to the tests.
@simcap do you mean the example of final usage with the stack-file
or without it?
Yeap, a full one liner or (multi liner) capturing the different way you do manage stack creation/ and update ... You might have that in your bash history (removing sensitive param if needed of course)
Anyway, I am just curious on how you manage the CI (i guess) pipeline with AWS and awless (even if I have some ideas). There might be stuff we could still improve or take into account. If you write an article about it let us know.
Yep, I'm using it with the CI. I'm planning to prepare slides about the process somewhere later in December, I will share it with you once it is ready.
Also, I will share usage example on the Issue comment, once finish with this PR.
Thanks. Examples are mostly to put in the changelog along the new feature.
Hey@simcap, now it should be ready for merge.
Thanks. If you confirm that you have tested live error cases (empty, rubbish, unexisting files) and they have a clear and helpful output, I will merge it today.
@simcap now it is ready, made errors a bit more user-friendly.
@simcap thanks 🙇
Hey,
I know you guys are busy with preparation for ReInvent, but I prepared small PR with
stack-file
support. No need to merge it now, but I will appreciate some review and feedback.One thing that is missing is the CLI usage/description for
stack-file
, since, AFAIK all the descriptions generated automatically from appropriate AWS documentation, butstack-file
not on the docs.BTW, in this PR many vendor files are removed, I think it is because you forgot to run
dep prune
implements Issue #145