pki-io / admin

Other
15 stars 7 forks source link

Registration flow #41

Closed zeroXten closed 9 years ago

zeroXten commented 9 years ago

Its a bit of a mess still. I keep mentally flipping between the CLI being an offline tool and the CLI being a prototype for how the API will work. The idea is to support an offline mode even when the API works, so it will probably just be a nice wrapper around the API functions.

Review on Reviewable

thanasisk commented 9 years ago

Comments from the review on Reviewable.io

Reviewed files:


zeroXten commented 9 years ago

Comments from the review on Reviewable.io


Ah ok. Does fmt.Errorf also call String()? I remember seeing a lot of ugly stuff around the errors when I just used err directly.


jonbonazza commented 9 years ago

Ill have to test, but i thought it did. Try it in the go playground and see. :)

On Mon, Jan 26, 2015, 5:21 PM Fraser Scott notifications@github.com wrote:

Comments from the review on Reviewable.io

https://reviewable.io:443/reviews/pki-io/admin/41

Ah ok. Does fmt.Errorf also call String()? I remember seeing a lot of ugly

stuff around the errors when I just used err directly.

— Reply to this email directly or view it on GitHub https://github.com/pki-io/admin/pull/41#issuecomment-71573601.

jonbonazza commented 9 years ago

Just tested. Yes it works.

On Mon, Jan 26, 2015, 5:24 PM Jon Bonazza jonbonazza@gmail.com wrote:

Ill have to test, but i thought it did. Try it in the go playground and see. :)

On Mon, Jan 26, 2015, 5:21 PM Fraser Scott notifications@github.com wrote:

Comments from the review on Reviewable.io

https://reviewable.io:443/reviews/pki-io/admin/41

Ah ok. Does fmt.Errorf also call String()? I remember seeing a lot of

ugly stuff around the errors when I just used err directly.

— Reply to this email directly or view it on GitHub https://github.com/pki-io/admin/pull/41#issuecomment-71573601.

zeroXten commented 9 years ago

Comments from the review on Reviewable.io


Ah cool. Will using the logging library change significantly how we use errors anyway?


jonbonazza commented 9 years ago

Kind of. We will still use fmt.Error() and such to create error objects to return, but the end consumer app will use the seelog package instead of the log package to write the log. Ill create a task to setup seelog for us and replace the current logging statements with the seelog package. There is an idiomatic way to do this, so i'll assign the task to me to start. :)

On Mon, Jan 26, 2015, 5:33 PM Fraser Scott notifications@github.com wrote:

Comments from the review on Reviewable.io

https://reviewable.io:443/reviews/pki-io/admin/41

Ah cool. Will using the logging library change significantly how we use

errors anyway?

— Reply to this email directly or view it on GitHub https://github.com/pki-io/admin/pull/41#issuecomment-71574733.

jonbonazza commented 9 years ago

Went ahead and merged. Making some changes to this package anyway for logging, so I will just make the changes I mentioned. =)