Closed alechenninger closed 9 years ago
Is it really necessary to include angular-strap
in the source? This type of thing gets out of date easily. If it has to be bundled, couldn't we do an overlay or explode the referenced artifact as part of the build process?
The way bower works is it just clones a git repo, so there's no bundled download AFAIK. It's definitely ugly, but I was waiting on https://github.com/bower/bower/issues/505 to be fixed to stop storing deps into source control (which is not uncommon unfortunately in this case). It's nice to be able to use semantic version ranges in your bower.json dependency declarations, but these means if building downloads dependencies each time, it may get different versions on different builds, which is not really good either. Checking it into source control ensures collaborators are all using the exact same versions of dependencies.
I can look into alternative solutions until the shrinkwrap feature is implemented.
Looks like I messed up the POM... will fix this this weekend.
Moved to in progress. Comment when it's ready for review again please @alechenninger
@jewzaam @derek63 Ready for review.
At this point there is no good solution I am aware of to avoid checking bower dependencies into source control, while still making sure that developers who check out the code base are using the same dependency versions (it is impossible to nail down transitive dependency versions due to semver version ranges). The aforementioned shrinkwrap feature that bower is working on should solve this when it is done, and if there's anything else we can do I'd like to put it off as a future refactoring.
However, to facilitate review, I removed the dependencies from this pull request. I think though that they should be checked back in before we merge this PR however. So please comment when review is done.
If those dependencies are required for this PR to be merged add them back in. They can ignored for the review.
Added back -- fair warning, though, it chokes up my browser a bit looking at the review page since it is unfortunately so very large. If bower doesn't implement the shrinkwrap feature soon I might have to take a stab at it myself!! :)
Something I noticed: The 'Data host' label is used for both columns on /app/data/#/environments page. The other column should be called 'Metadata host'. Other than that, looks good.
I would be nice to hide bower_components changes from the diff, but as long as 3rd party code is in separate commit, it's still reviewable.
With this, the application is completely decoupled from a server side component. In the current state, all that's needed is for a consumer to download the application; the rest is just Javascript. For instance, you could even run the app by cloning the repo and serving the directory on your on machine if you had connectivity available to the lightblue hosts. It makes development on the app very fluid.
Summary of changes: