Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

miscellaneous fixes #363

Merged
merged 16 commits into from
Nov 25, 2014
Merged

miscellaneous fixes #363

merged 16 commits into from
Nov 25, 2014

Conversation

btc
Copy link
Contributor

@btc btc commented Nov 18, 2014

This is a shared branch containing improvements too small for individual PRs.

Feel free to push to it. It'll be merged before the end of the week.

@btc btc added the status/in-progress In progress label Nov 18, 2014
@btc btc added codereview and removed status/in-progress In progress labels Nov 24, 2014
Brian Tiger Chow and others added 16 commits November 25, 2014 06:12
refactor(config) use util method
4831ca6 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
log(core) bootstrapping error -> critical
2497c93 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
@mappum
commands/http: Allow API requests from whitelisted origins
aaab7d0
@mappum
daemon: Set allowed API origins in API_ORIGIN environment variable
706ebe8
@jbenet
importer: simplified splitter
f6235c5 
The splitter is simplified using io.ReadFull, as this function
does exactly what we wanted.

I believe io.ErrUnexpectedEOF should be handled as an EOF here,
but please correct me if I'm wrong.
fix(bitswap/notifications) don't force sender to block on receiver
3fdbd6c 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
fix(bitswap) shutdown
72a0cc1 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
refactor(eventlog) extract Loggable
08d709d 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>

# TYPES
# feat
# fix
# docs
# style (formatting, missing semi colons, etc; no code change):
# refactor
# test (adding missing tests, refactoring tests; no production code change)
# chore (updating grunt tasks etc; no production code change)
refactor(eventlog) rename ContextWithLoggable
58d299e 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>

fix(eventlog) compilation error

License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
docs(eventlog)
247fd67 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
log(dht) add eventlog.Update event
b8c96b2 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
feat(main) create root context
b5dc686 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
log(main) add log event for executing pre-command hooks
9e6177e 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
log(core) Event: bootstrap
f3b71cf 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
log(dht) Event: connect
3ed9b0c 
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
log(swarm, netmessage) Event: connSetup(begin|success)
0fc95d5 
+ netmessage is now loggable

License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
@btc
Copy link
Contributor Author

btc commented Nov 25, 2014

@jbenet @whyrusleeping LGTU?

@whyrusleeping
Copy link
Member

yeah LGTM

jbenet
jbenet reviewed Nov 25, 2014
View reviewed changes
core/core.go
)

const IpnsValidatorTag = "ipns"

var log = u.Logger("core")
var log = eventlog.Logger("core")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we get to completely removing these .Logger(...) lines (i.e. import eventlog as log and use it directly?)? can eventlog grab the package name? (or even the filepath?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what the best path is, but this var log = pkg.Logger(..) thing isn't adding much

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eventlog cannot grab the package name.

but this var log = pkg.Logger(..) thing isn't adding much

the package name... it adds precisely that. I haven't yet found a simple way to achieve this by other means.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could write some SUPER ugly reflection code that grabs the call stack, analyses the caller function and queries which package its in... Other than that we are stuck with this approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're referring to runtme.FuncForPC, it's not possible to get the package name.

@jbenet
Copy link
Member

jbenet commented Nov 25, 2014

👍 LGTM. RTM

btc pushed a commit that referenced this pull request Nov 25, 2014
Merge pull request #363 from jbenet/misc/2014-10-2X
bad4db3 
miscellaneous fixes
@btc btc merged commit bad4db3 into master Nov 25, 2014
@btc btc removed the codereview label Nov 25, 2014
@btc btc deleted the misc/2014-10-2X branch November 25, 2014 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@btc @whyrusleeping @jbenet @mappum