Notes on `slog` usage. about rethinkdb-rs HOT 6 CLOSED

No worries. We all do our projects when we have free time. I understand, and thanks for the head ups.

from rethinkdb-rs.

Thank you for taking some time off your busy schedule to review my code. I'm sorry for taking so long to get back to you. I thought I would have replied by now but I have been awfully busy. Since I don't want to rush my response to this and I would like to take this opportunity to seek further guidance from you, I will give you a shout once I'm ready. I really appreciate your feedback. I have already addressed some of your points. Thank you, once again.

from rethinkdb-rs.

I think I have now addressed all your points so I am closing this now. However, I would really appreciate it if you can review the code again. I know you are very busy so don't feel bad at all if you can't. I will totally understand that. You have helped me a lot as it is already. I have responded to your original comments below but just to explain why I did it the way I did it at the time. The new code is almost a full re-write.

I'm the author of slog.

I already knew who you were by that time so you didn't really need to introduce yourself 😉 I'm a huge fan of your work.

I hope it (slog) will work well for you.

I think it's working very well for me so far 😄 I love it! Right now I'm working towards releasing v0.0.6 within the next 24 hours. I would have loved to use slog v2 but unfortunately I couldn't get slog-term (v2 alpha) to work with it. I need slog-term for the examples.

Logger is Sync so it does not require wrapping in RwLock.

Yes I noticed that. The reason I put it in a RwLock at that time was so I could mutate it at runtime. The idea was to listen for certain signals on a running server (in production) and change the configuration of the logger accordingly.

Generally libraries should almost never create their own Drains.

For v0.0.5 and below logging was just for me to help with debugging as I developed the library (instead of using print statements). I don't think I even exposed it publicly at that time.

... thing is very non-idiomatic. reql is it's own crate, with it's own namespace.

This wasn't about namespacing. I know the docs said "The top-level ReQL namespace" but that was just because I was trying to be consistent with the documentation of the official drivers. The client object for ReQL is called r by convention and for convenience it comes defined by all the official drivers including the Java driver which, like Rust, already has namespaces baked in. I was trying to do the same thing for convenience and consistency with the official drivers.

I'm not sure if logging errors in error! is a good idea. I mean - it's the data that is being returned to the user of the library and most of the time user is going to log an error themselves. It only makes sense for the library to log things that the user of it is not able to access directly, to allow transparency into what is happening internally.

I'm glad you actually noticed this because I may have valuable feedback for you here. It would be awesome if you could incorporate this feedback into slog somehow, if at all possible. The thing is, as an amateur Sysadmin I'm often overwhelmed by too much logging information. I find it counter-productive. Instead of helping me debug the problem at hand I find all the other logs distracting. The idea with that error macro was so that I could log only when an error occurred and also print only information that's helpful in debugging that particular error.

I hope you will find my feedback somehow useful. Once again, thank you very much for taking your time to review my work. I really appreciate that. Thank you for the helpful pointers and thank you for slog, it's amazing 👍

from rethinkdb-rs.

(v2 alpha) is not the right version. It was yanked. I think the latest one is 2.0.0-4.0 . Sorry for the confusion.

from rethinkdb-rs.

Oh wow, a lot have changed. There has been a lot of effort put into this crate. This was one of the first crates using slog so it was important for me to be able to show it as an example, which I'm going to do eagerly now. :) Great job.

from rethinkdb-rs.

Thank you. I'm glad you like the new changes.

from rethinkdb-rs.