search

hapijs / good

hapi process monitoring
Other
525 stars 160 forks source link

Restore wreck logging #469 #478

Closed codyzu closed 8 years ago

codyzu commented 8 years ago

Fixes #469

This can be merged as it is working.

I reverted the commit #455 and attempted to improve the tests (see my file comments below)...

codyzu commented 8 years ago

any ideas why the ci is failing?

arb commented 8 years ago

This is not correct. Please see the associated issue and the wreck docs for the proper way to listen for these events.

Also, since #469 is not on the 7.0.0 milestone, this isn't going to be merged in the short term.

I appreciate your go-getter attitude though 😄

codyzu commented 8 years ago

@arb can you be a little more specific? Like I said, I just "re-added" the code that was deleted in #455.

I must be missing something, because I don't know what you mean by

the proper way to listen for these events

While the wreck docs say we can

const symbol = Symbol.for('wreck');
process[symbol].on('response', (err) => {
...

It also says that is when we want to...

capture a wreck event, after wreck has been loaded, but in a module that doesn't require wreck

(emphasis added by me)

Since we already have a reference to wreck, we can do Wreck.on AFAIK. Sorry to be needy, but can you be a little more specific :-) Ahh... you mean we need to add a way to disable this???

arb commented 8 years ago

I don't want that code re-added. I want it re-implemented per the wreck documentation with the symbol and we didn't have a reference to wreck until you added it in this PR. You just need to do the stuff for the "response" event explained here

arb commented 8 years ago

@codyzu are you still interested in landing this? If not, I'm going to close and let someone else work on this.

codyzu commented 8 years ago

@arb I made a little more progress this, but truthfully I'm not sure when I will find the time to finish this in the near term, so if someone else wants to take this on, no worries on my side.

In general, I really want to get involved in hapi. I chose this issue because it seemed like way a good way to get started... But in the end it seems I didn't understand all of the details (I have really no experience with wreck). If you have any suggestions for another way for me to get involved in the project, please don't hesitate to send me some info.

arb commented 8 years ago

I'll close this PR for now and if you get time in the future, you can open a new one. Like I said above, check the associated wreck issue and the wreck documentation; it tells you exactly how to do this using a JavaScript Symbol object and the global process object.

A good way to always get started is contribute documentation updates as that is often the least maintained part of any open source project.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.