bower / registry

The Bower registry
https://registry.bower.io/packages
MIT License
292 stars 67 forks source link

make registry.go more idiomatic #188

Closed bystones closed 8 years ago

bystones commented 8 years ago

Completly untested, so use with caution.

I left two TODOs in there:

  1. I'm not sure if the defer on the node process does something. Does is wait for the node process to stop when the main program exits? Not sure.
  2. Is ffjson with the map faster instead of using the standard lib with a struct? You could drop a dependency this way. Or use ffjson with a struct and generate the mapping file.
sheerun commented 8 years ago
  1. I used defer to make Wait call asynchronous, so we can run both node server and go proxy processes.
  2. I think it's slightly faster, I don't want to drop this dependency. I have no experience in go, so maybe you'd like to fix it with struct?
bystones commented 8 years ago
  1. defer means the function is run after the containing function exits. "go" runs the function concurrently. cmd.Start already runs the process concurrently. Wait just waits for it to finish (https://golang.org/pkg/os/exec/#Cmd.Wait). So you could drop this part or use go and some logic to restart node if it fails.
  2. How much faster is it and do you need it to be faster? I personally would use the std lib as much as possible unless you really need to speed. To have that argument there should be some benchmark to show the speed difference.
bystones commented 8 years ago

I added a few more commits on top with dropping ffjson for encoding/json.

I don't have Postgres installed so I didn't run tests. If you want to take the commit which drops ffjson you need to update the Godeps file as well (never used that tool so I didn't do that either).

dgryski commented 8 years ago

I did some benchmarking. The big json speedup is moving from a map to a struct. The json->ffjson step doesn't make much difference. I recommend sticking with the standard library for now.

sheerun commented 8 years ago

I've deployed the code and there isn't really a difference: https://www.dropbox.com/s/mu2c19vi0s0adbw/Screenshot%202016-04-08%2017.07.09.png?dl=0

The code is cleaner though, so thank you :)