Follow-up for the issue I created: https://github.com/MaxCDN/bootstrap-cdn/issues/460

Instead of statically providing the supported CDN versions for both Bootstrap and Bootswatch, we should instead populate them dynamically via the jsdelivr API.

CommentFileSizeAuthor
#20 provide_a_more_robust-2450757-20.patch99.46 KBneardark
#9 bootstrap-provide-a-more-robust-2450757-9.patch78.63 KBmarkhalliwell
#4 bootstrap-populate-available-2450757-3.patch16.29 KBmarkhalliwell
#3 bootstrap-populate-available-2450757-3.patch5.37 KBmarkhalliwell

Comments

waverate’s picture

waverate commented

Mark,

This looks good for Bootstrap:

http://maxcdn.bootstrapcdn.com/data/bootstrapcdn.json

markhalliwell’s picture

markhalliwell
Primary language English
commented

That's just for Bootstrap. In the GH issue I linked, I learned that file is automatically generated from http://api.jsdelivr.com/v1/bootstrap/libraries. Considering that this JSON data has both Bootstrap and Bootswatch, I think it'd be better to use that one directly.

markhalliwell’s picture

markhalliwell
Primary language English
commented
StatusFileSize
new bootstrap-populate-available-2450757-3.patch5.37 KB

Initial WIP (non-working) patch of dynamic CDN

markhalliwell’s picture

markhalliwell
Primary language English
commented
StatusFileSize
new bootstrap-populate-available-2450757-3.patch16.29 KB

Sigh... it didn't add the new file... trying again...

markhalliwell’s picture

markhalliwell
Primary language English
commented
Title: Populate available CDN versions via jsdelivr API » Provide a more robust CDN solution
waverate’s picture

waverate commented

Mark,

Against latest 7.x-3.x-dev, I get a WSOD on the settings page. From log:

Notice: Use of undefined constant BOOTSTRAP_MAJOR_VERSION - assumed 'BOOTSTRAP_MAJOR_VERSION' in _bootstrap_cdn_api() (line 113 of sites/all/themes/bootstrap/includes/cdn.inc).

Using php 5.4.38

Christopher Riley’s picture

Christopher Riley commented

I ran into the same issue. It appears that it is not reading the define out of template.php. I added the same define to the top of includes/cdn.php and it works and does not give a already defined message. I added a line to the top of template.php saying it was loaded and another at the top of cdn.php and I see the message about cdn.php but not template.php.

How does the loading order work with a subtheme? Could this be a subtheme related issue? For the time being I moved the define over to cdn.php and all is working but would love to see this cleaned up and implemented.

Thanks in advance.

markhalliwell’s picture

markhalliwell
Primary language English
commented

WIP (non-working) patch

I would have thought this was pretty self-explanatory...

markhalliwell’s picture

markhalliwell
Primary language English
commented
StatusFileSize
new bootstrap-provide-a-more-robust-2450757-9.patch78.63 KB

Final patch

markhalliwell’s picture

markhalliwell
Primary language English
commented
Assigned: Unassigned » neardark
Status: Active » Needs review

Ok. I think this is "done" and ready for review/testing.

@neardark, please advise. Doesn't have to be ported to 8.x just yet. Just need as many eyes as possible on this.

markhalliwell’s picture

markhalliwell
Primary language English
commented
neardark’s picture

neardark commented

Only two minor minor items noted below. I need to re-review when my mind is clear, but this looks great. I did install, and it worked well. 

  1. +++ b/includes/cdn.inc
@@ -0,0 +1,490 @@
+ *   $provider is not set. If $provider is set and exists, it's individual

+++ b/includes/common.inc
@@ -104,6 +173,42 @@ function bootstrap_include($theme, $path) {
+  if (isset($deprecated[$prefix . $name]) && ($setting = theme_get_setting($deprecated[$prefix . $name], $theme))) {

@@ -900,7 +1005,7 @@ function _bootstrap_tooltip_description($description) {
 }

    it's should be its 

  2. +++ b/includes/cdn.inc
@@ -0,0 +1,490 @@
+            '!drupal_http_request' => 'https://api.drupal.org/api/drupal/includes%21common.inc/function/drupal_http_request/7',

    %21 should be a forward slash I believe.

waverate’s picture

waverate commented

Patch #9 with #12 changes works.

  • markcarver committed a6c45bc on 7.x-3.x 
    Issue #2450757 by markcarver: Provide a more robust CDN solution
markhalliwell’s picture

markhalliwell
Primary language English
commented
Version: 7.x-3.x-dev » 8.x-3.x-dev
Status: Needs review » Patch (to be ported)

Ok, I addressed #12.1. #12.2 is correct, API doesn't recognize a forward slash there.

Moving this to 8.x-3.x for port and more review. If there's any further issues found with this work, please open a new issue to address.

I'll be creating a change record for this issue.

markhalliwell’s picture

markhalliwell
Primary language English
commented

Published change record: https://www.drupal.org/node/2452617

  • markcarver committed 31afd0c on 7.x-3.x 
    Issue #2450757 by markcarver: Provide a more robust CDN solution

Minor...

  • markcarver committed a6722d9 on 7.x-3.x 
    Issue #2450757 by markcarver: Provide a more robust CDN solution

Fix...

  • markcarver committed 1427c5f on 7.x-3.x 
    Issue #2450757 by markcarver: Provide a more robust CDN solution

Fixed...
neardark’s picture

neardark commented
Assigned: neardark » Unassigned
Status: Patch (to be ported) » Needs review
StatusFileSize
new provide_a_more_robust-2450757-20.patch99.46 KB

  • neardark committed 1a8dedb on 8.x-3.x 
    Issue #2450757 by markcarver, neardark: Provide a more robust CDN...
  • neardark committed 961a3f6 on 8.x-3.x
    Revert "Issue #2450757 by markcarver, neardark: Provide a more robust...
  • neardark committed cf2dccc on 8.x-3.x
    Issue #2450757 by markcarver, neardark: Provide a more robust CDN...
neardark’s picture

neardark commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

pol’s picture

pol
Location Brussels
commented

Hi all,

I made a Libraries API integration module with CDN, you can find it here: https://www.drupal.org/project/libraries_cdn
It might certainly help you managing your CDN in a easy way.