sourcegraph / srclib-go

Go toolchain for srclib
https://sourcegraph.com/sourcegraph/srclib-go
MIT License
29 stars 13 forks source link

Work towards a better architecture #96

Closed neelance closed 8 years ago

neelance commented 8 years ago

Two main goals:

keegancsmith commented 8 years ago

Love the changes here. Especially the later refactorings to ast visitor.

I assume we can merge this in now? Can we give this PR a real title + record somewhere the motivation for why we are removing loader.

Also a lot of the commits here would benefit from a message in the commit message body explaining why a change is being made or why it is safe to do the change.

LGTM!

neelance commented 8 years ago

I've improved title and description, then merged. I will try in the future to include more context in the commit messages.

dmitshur commented 8 years ago

I just want to echo this:

Also a lot of the commits here would benefit from a message in the commit message body explaining why a change is being made or why it is safe to do the change.

I agree that is very, very helpful and much appreciated!

I will try in the future to include more context in the commit messages.

Thanks. :)

This looks like an awesome change!