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.

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

Use global variable for CDN root directory #5333

Merged
merged 2 commits into from
Jul 17, 2023

Conversation

diarmidmackenzie
Copy link
Contributor

@diarmidmackenzie diarmidmackenzie commented Jul 9, 2023

Description:

As per #5119

Changes proposed:

Replace hardcoded instances of https://cdn.aframe.io/ with a global variable AFRAME_ROOT_CDN.

Based on @vincentfretin's suggestion here

Copy link
Contributor

@vincentfretin vincentfretin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this work.

src/constants/index.js Outdated Show resolved Hide resolved
@vincentfretin
Copy link
Contributor

@diarmidmackenzie
Copy link
Contributor Author

The text in https://github.com/aframevr/aframe/blob/master/docs/introduction/faq.md#can-i-use-a-frame-offline-or-self-hosted would also need to be modified.

I'd like to handle that as a separate PR.

I don't think the text as it stands needs to change for this PR to be OK - everything it says is still true, even after this code change.

More work needed to fully solve #5119, but I'd like to tackle this one bit at a time.

@nightgryphon
Copy link
Contributor

it also be cool to add an assets archive for loacal deployment. Including fonts, controllers models etc. I has to manualy figure out which files are required for local deployment and download one by one...

@vincentfretin
Copy link
Contributor

@nightgryphon You can clone the assets repo actually, I commented about it here #5119 (comment)

@nightgryphon
Copy link
Contributor

@vincentfretin Ohh! I've missed that repo. It can be a good idea to add a comment line with repo url near the constant and later in the docs. Also it is good to note which folders are actually required.

@dmarcos
Copy link
Member

dmarcos commented Jul 17, 2023

Thanks!

@dmarcos dmarcos merged commit 3246f0c into aframevr:master Jul 17, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants