Code improvements - Githubissues

rock-core / vscode-rock

VSCode extension for Rock integration

MIT License

1 stars 1 forks source link

Code improvements #20

Closed doudou closed 6 years ago

doudou commented 6 years ago

The first change is to get rid of most of the Demeter chains that do not make sense. This is really because I'm getting headaches when I try to follow the mock chains. The good side effect IMO is that it improves the interfaces.

Other changes:

WorkspaceInfo uses the package directory as key
RockPackage subclasses contain the autoproj.Package as field.

g-arjones commented 6 years ago

I like most of the changes and my comments are mostly regarding design (we seem to have different plans), so the comments are just points for you to consider. Since I don't see any major problem with the changes, feel free to merge 👍

doudou commented 6 years ago

I've implemented your suggestions. For the pickers, I'd like to leave them where they are right now, but let's keep in mind that we could move them back into Package if it starts to look better.

g-arjones commented 6 years ago

Ok.. Looks good. We could also refactor autoproj.Workspaces to make the maps private and create acessors to inspect the packages and workspaces directly:

getWorkspaceByRoot(root: string);
getWorkspaceByPackage(path: string);
getPackageInfo(path: string);
...

This requires a lot of refactoring though so maybe we should leave it for another PR. I'm fine with merging this one.. 👍