Remove or document references to openils_dojo.js

Bug #1076582 reported by edoceo
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Low
Unassigned

Bug Description

In 25 locations are references to:

  /js/dojo/dojo/openils_dojo.js

Which no longer exists in Evergreen and it makes a bunch of 404s in my web-server logs

Not critical, just annoying.

Evergreen: master 2e10326b9e0e
OpenSRF: master

Revision history for this message
edoceo (edoceo) wrote :

Reject/Close it, sorry.

This is part of the secret sauce:
http://evergreen-ils.org/dokuwiki/doku.php?id=scratchpad:random_magic_spells#custom_dojo_build

So, perhaps some documentation improvements are in order.

Revision history for this message
edoceo (edoceo) wrote :
tags: added: pullrequest
Revision history for this message
edoceo (edoceo) wrote :

For those interested, here's some home page (/opac/en-US/skin/default/xml/index.xml) differences I measured, test environment.

Pre Dojo Build
---
128 requests ❘ 84.92KB transferred ❘ 668ms (onload: 673ms, DOMContentLoaded: 650ms)
128 requests ❘ 84.92KB transferred ❘ 575ms (onload: 581ms, DOMContentLoaded: 565ms)
128 requests ❘ 84.92KB transferred ❘ 623ms (onload: 628ms, DOMContentLoaded: 604ms)
---

Post Dojo Build
---
89 requests ❘ 77.59KB transferred ❘ 727ms (onload: 736ms, DOMContentLoaded: 710ms)
89 requests ❘ 77.60KB transferred ❘ 729ms (onload: 736ms, DOMContentLoaded: 706ms)
89 requests ❘ 77.59KB transferred ❘ 708ms (onload: 713ms, DOMContentLoaded: 689ms)
---

Fewer requests, less total bytes, small increase in load/render.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I find the description and comments a bit confusing, though I think I know what is going on here. However, I want to ask for clarification:

You are saying that the documentation needs to be updated to be clear about custom dojo builds? Your branch on #2 provides a possible update?

Changed in evergreen:
status: New → Incomplete
Revision history for this message
edoceo (edoceo) wrote :

Correct, comment #2 patch is an update to the docs directory, with a few notes on how to do it, #3 is some tech details.

The longer part of the story is that while using Evergreen I noticed all these 404s of which this was one. That confused me, I ack'd the code for that text, found numerous references and then heard a rumour it was not in use. Only later did I learn that it was for doing a custom build with dojo; when I added comment #1;

It wasn't until just this very instant I noticed an edit button on the issue description, so perhaps I should update that.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Thanks for the clarification!

Changed in evergreen:
status: Incomplete → Confirmed
importance: Undecided → Medium
importance: Medium → Low
Revision history for this message
Mike Rylander (mrylander) wrote :

I'd personally rather see the packaging scripts build a custom dojo layer (or pull one we pre-build) since the transport cost savings can be substantial. IIRC, it saves over 0.5MB for dojo-heavy pages.

Ben Shum (bshum)
Changed in evergreen:
status: Confirmed → Opinion
tags: removed: pullrequest
Revision history for this message
Ben Shum (bshum) wrote : Re: Documentation explaining openils_dojo.js

Changing the title of this bug since it is now about adding documentation for explaining openils_dojo.js and how it gets created.

If Mike or others would like to create code for automatically creating the file, we can start a new bug, but for now, let's make this more visible again.

summary: - Orphan references to openils_dojo.js should be removed
+ Documentation explaining openils_dojo.js
Changed in evergreen:
status: Opinion → Confirmed
tags: added: documentation
Revision history for this message
Jason Stephenson (jstephenson) wrote :

And, today, I find myself wondering what is the purpose of openils_dojo.js and friends.

This comes now because when we switched over to new hardware, our shelving location box on advanced search seems to work intermittently. Looking into that I discovered a console log error about openils_dojo.js being missing, but it doesn't seem to affect the performance of the OPAC, really.

I wonder if the discussion should be reopened to remove the references or should code be added as part of the installation to create the missing files?

Revision history for this message
Chris Sharp (chrissharp123) wrote :

I just stumbled on this and filed duplicate bug 1617556. My branch that removes references to openils_dojo.js is here:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1617556_remove_openils_dojo_references

Revision history for this message
Andrea Neiman (aneiman) wrote :

removing docs tag, as Chris's branch proposes removal of the dojo references & as we move farther from dojo there's no real need to document it.

tags: removed: documentation
Revision history for this message
Terran McCanna (tmccanna) wrote :

Not 100% certain this is still valid, but if it is, it's worth brushing off.

tags: added: cleanup
tags: added: pullrequest
tags: added: needsrepatch
Changed in evergreen:
assignee: nobody → Jeff Davis (jdavis-sitka)
Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

It's still valid in that the unnecessary 404 errors still happen. I don't think the solution is to document it, since we are working on getting rid of Dojo. I think references to openils_dojo.js should be removed from the code, and the few sites that want to continue to use the optional optimization can maintain it as a local customization.

Accordingly, working branch user/jeffdavis/lp1617556_remove_openils_dojo_references takes Chris's old branch and applies it to current master, and removes an additional reference to openils_dojo.js that snuck into the Bootstrap OPAC:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jeffdavis/lp1617556_remove_openils_dojo_references

tags: removed: needsrepatch
Changed in evergreen:
assignee: Jeff Davis (jdavis-sitka) → nobody
milestone: none → 3.7-beta
summary: - Documentation explaining openils_dojo.js
+ Remove or document references to openils_dojo.js
Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Jeff Davis, could you split your changes into a separate commit from Chris Sharp's in your branch, please?

Splitting the changes into two commits makes it easier to back port this code, for sites that may want to do so. It will also make it easier to tell who changed what for purposes of attribution.

Thanks,
Jason Stephenson

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

Branch user/jeffdavis/lp1617556_remove_openils_dojo_references_2 splits the changes into two commits, per above.

Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Fix committed to master for progress in eliminating Dojo from Evergreen.

I also added a release note and removed the openils.profile.js required to make the custom Dojo in an additional commit.

Thanks, everyone who participated in this bug!

Changed in evergreen:
status: Confirmed → Fix Committed
assignee: Jason Stephenson (jstephenson) → nobody
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers