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

Add bower support #440

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add bower support #440

wants to merge 9 commits into from

Conversation

titilambert
Copy link
Contributor

Hello !
Here the bower support for Adagios :)

@matthieucan Do you see any error ?

Thanks !

@matthieucan @titilambert
remove embedded js/css libraries
27081cb
@matthieucan @titilambert
hello bower
9d64227
@matthieucan @titilambert
update front-end dependencies
63f9cdb
@matthieucan @titilambert
update paths for use with bower
7a3aa08 
Conflicts:
	adagios/bi/templates/business_process_standalone.html
	adagios/bi/templates/business_process_view.html
	adagios/media/css/import_contrib.css
	adagios/objectbrowser/templates/list_object_types.html
	adagios/status/templates/base_status.html
	adagios/status/templates/status_detail.html
	adagios/status/templates/status_index.html
	adagios/templates/base.html
@matthieucan @titilambert
update readme bower
44fc6b1
@matthieucan @titilambert
add: bower instructions in the install process
04190db
@matthieucan @titilambert
fix bootstrap-datepicker bower dependency
81e1d6e 
Conflicts:
	adagios/contrib/lib/dashboards/raymii.html
	adagios/templates/base.html
@matthieucan @titilambert
fix: dependency url
e6c8018
@matthieucan
Copy link
Contributor

If bower.json is the same as in savoirfairelinux/adagios, the rest will follow :)

@titilambert
Copy link
Contributor Author

@matthieucan Yes it is :)

@tomas-edwardsson
Copy link
Contributor

This pull request seems to have some issues. I think the main issue is that there are javascript libarary updates within this pull request. One problem noted is that the objectbrowser (Configure) list table does not render at all. Might have something to do with datatables being upgraded to 1.10.2.

Adagios versions

package master bower notes
bootstrap 2.0.4 2.3.2
chosen 0.9.8 0.9.8
datatables 1.9.2 1.10.2
bootstrap-datepicker unknown 1.3.0 the original doesn't seem to be versioned
jquery 1.9.1 1.9.1
select2 3.2 head Should probably reference a release
datatable None head New, should probably reference a release, belongs in a different pull request?

This needs some more testing before we can merge.

We cannot update each library in independant pull requests since the libraries have versioned dependencies into each other and will not work without updating the others.

@titilambert
Copy link
Contributor Author

@matthieucan I hope you can REhelp me with this one

@matthieucan
Copy link
Contributor

@titilambert does it work when you downgrade datatables?

@palli
Copy link
Contributor

palli commented Oct 23, 2014

We need a test for object browser.configure... It seems to be one of the
most fragile things that are currently untested.
On Oct 23, 2014 4:04 PM, "Matthieu Caneill" [email protected]
wrote:

@titilambert https://github.com/titilambert does it work when you
downgrade datatables?

—
Reply to this email directly or view it on GitHub
#440 (comment).

@titilambert
Copy link
Contributor Author

@matthieucan In fact, it's only DT_bootstrap which is missing...
You already wrote it in the README.bower file ...
Could you tell us how to fix ALL what you wrote in the README.bower file ?

Thanks !

@matthieucan
Copy link
Contributor

  • html5shim: that's an external dependecy downloaded directly via a CDN, this fact might not suite your free software requirements; if not you'll have to patch Bootstrap and add it in the Bower list.
  • jqplot: maybe have a direct link to the source file; but this library was only used for demos purposes and is not needed at all if you don't use the "plot" template.
  • DT_bootstrap.{css,js}: well you added its source code. If upstream doesn't ship the files anymore, there's barely a better solution!

@titilambert
Copy link
Contributor Author

@tomas-edwardsson I think I resolve the issue with the last commit

@palli palli mentioned this pull request Oct 25, 2014
Merged
@palli
Copy link
Contributor

palli commented Oct 27, 2014

This patch is making me uncomfortably aware of how many (shaky?) javascript dependencies we have.

Does anyone have opinion on the dependencies that are outside bower repositories (like select2 which points to a github repository).

There is a real risk, that 1 or 2 years in the future those links might be gone, effectively breaking adagios.

A workaround could be to fork those repositories and point our links there.

@matthieucan
Copy link
Contributor

@palli keep in mind most bower repos are github repos!

@palli
Copy link
Contributor

palli commented Oct 27, 2014

yes, which helps us partly if (when) any of them updates and breaks backwards compatibility, but not if the projects are removed or renamed.

@matthieucan
Copy link
Contributor

Sure, the idea of forking them is nice, and doesn't prevent to still linking to the original repos :)

@tomas-edwardsson
Copy link
Contributor

I don't think this is a real issue and we should cross that bridge when we need to. The libraries that we are using are packaged in the rpm/deb packages so if any of them are removed/disappear we can find the source there. I really want to avoid having to maintain forks if it is not needed.

@palli
Copy link
Contributor

palli commented Nov 9, 2014

Are you still working on this patch ? I noticed a few issues that might block this patch from getting into master, namely:

  • Bootstrap is being updated from 2.0.4 to 2.3.2 and causes styling problems
  • Bootstrap-3.1.1 is removed (was used by contrib/raymii.html)
  • javascript errors happen on page load (at least I noticed DT_bootstrap.js was missing)

Going forward, i'm pretty sure we need something like bower to keep our javascript dependencies maintainable, but maybe this was to big of a change in one point in time. Also I think it is a general problem that we don't have a copy of them locally as well.

I propose the following:

  • Move all external javscripts into a sub-module (already done in Move adagios/media/externals into a sub-module #458)
  • Make only a proof-of-concept version of bower.json which has only jquery as a depency.
  • I'm thinking with this new setup bower.json maybe belongs in adagios-dependencies repository instead of adagios.repository.
  • When this is done, we look at bowerising other dependencies as well, making sure not to create any breakage

This way when doing git clone of adagios, we are guaranteed to have a working version of adagios because a downloaded copy of the dependencies exists in media/external

Comments ?

@titilambert
Copy link
Contributor Author

Hello !
I can not work on this patch right now... Maybe in January.
I think what you proposed is good for now.
So, let go like this !
Thanks !

Do you want I close this issue ?

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
@titilambert @matthieucan @tomas-edwardsson @palli