search

apache / accumulo-website

Apache Accumulo Website
https://accumulo.apache.org
14 stars 100 forks source link

Add bootstrap map files #426

Closed DomGarguilo closed 1 month ago

DomGarguilo commented 3 months ago

Fixes #425

Adds the map files for the minified boostrap resources added in #423

These map files were download from https://github.com/twbs/bootstrap/archive/v5.3.1.zip

DomGarguilo commented 1 month ago

In a27b55f I replaced the .min and .map files with the single non-minified versions. I also took this opportunity to bump to bootstrap version 5.3.3

DomGarguilo commented 1 month ago

These changes look good to me. Two questions I have are:

  • Does this resolve the issue you were seeing?

Yes, the error I was seeing is no longer present.

  • Is it worth changing the other minified resources to their regular versions also?

It is an easy enough change to make so I could convert the others too as part of a version bump in a separate PR.

DomGarguilo commented 1 month ago

These changes look good to me. Two questions I have are:

  • Does this resolve the issue you were seeing?

Yes, the error I was seeing is no longer present.

I guess I spoke too soon. After a while of the development docker container running, I still see the original errors (except now they mention the non minified versions):

[2024-07-17 23:48:45] ERROR `/css/bootstrap/5.3.3/dist/css/bootstrap.css.map' not found.
[2024-07-17 23:48:45] ERROR `/js/bootstrap/5.3.3/dist/js/bootstrap.bundle.js.map' not found.
ctubbsii commented 1 month ago

Hmm, that's very weird. I'd like to try to reproduce this. I have no idea what is creating these errors. The files themselves are definitely not needed.

DomGarguilo commented 1 month ago

Hmm, that's very weird. I'd like to try to reproduce this. I have no idea what is creating these errors. The files themselves are definitely not needed.

I am running things in the development docker container. Should pop up there. The files are refferenced at the bottom of the files. For example: https://github.com/apache/accumulo-website/blob/1b64cd852624e0ebb47fb15d6da315d4b911292b/js/bootstrap/5.3.3/dist/js/bootstrap.bundle.js#L6314

It has the same for the non minified too.

Edit: If you want to reproduce with the regular files (not .min you can checkout my branch: https://github.com/DomGarguilo/accumulo-website/tree/nonMinifiedResources).

One option to stop seeing these errors would be to remove the comments in the files mentioned above but I am not sure if we want to modify the files.

ctubbsii commented 1 month ago

Hmm, that's very weird. I'd like to try to reproduce this. I have no idea what is creating these errors. The files themselves are definitely not needed.

I am running things in the development docker container. Should pop up there. The files are refferenced at the bottom of the files. For example:

The Dockerfile just seems to run bundle exec jekyll serve. I don't get any errors when I run that locally, so I'm very curious what's different. Perhaps it's a different version of jekyll, or some dependency? I'm not sure where the errors are appearing, or which command they are coming from. I don't have Docker set up, so I can't reproduce this.

I did find https://stackoverflow.com/questions/57582243/no-sourcemap-for-css This might be some difference in default behavior coming from different versions of jekyll, or sass, or something. Perhaps we can force one behavior or another by setting configuration?

ctubbsii commented 1 month ago

Also, perhaps we can just ignore the errors? They seem to be false positives of some sort anyway. I don't think they represent an actual error in anything.

DomGarguilo commented 3 weeks ago

Also, perhaps we can just ignore the errors? They seem to be false positives of some sort anyway. I don't think they represent an actual error in anything.

Yea it should be fine to ignore them. Especially if they are only appearing in the development docker output.