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

Update .gitignore to ignore dist folder #47

Closed
john-Graham opened this issue Jul 11, 2018 · 4 comments
Closed

Update .gitignore to ignore dist folder #47

john-Graham opened this issue Jul 11, 2018 · 4 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@john-Graham
Copy link
Contributor

Please summarize your question in one sentence

I'm curious mostly the .gitignore is excluding

  • dist/css/style.min.css
  • dist/css/bootstrap.min.css

shouldn't we just as well ignore the whole folder?

dist/

Give a more extended description

Steps to reproduce (if needed)

Other comments or remarks

@jordanpinski
Copy link

The /dist folder contains more than those two files.

@david-poindexter
Copy link
Member

This has sparked a nice healthy internal conversation here and we believe some re-architecture is warranted. Currently only the required files are in the /dist folder out-of-the-box. Mainly these are files supporting Bootstrap and Font Awesome. We discussed and agreed these file should actually be in a source folder somewhere (TBD) and we'll rely on the gulp build process to place them appropriately in the /dist folder. Once that is completed, we can absolutely ignore the entire /dist folder via .gitignore. Thanks for indirectly bringing this to light!

@david-poindexter david-poindexter added the enhancement New feature or request label Jul 11, 2018
@john-Graham
Copy link
Contributor Author

Excellent. That'd be great. I ended up dumping BS4 and switched it to CSS Grid, and upgraded to FA5 and added that to part in the _includes and referenced the CDN.

<dnn:DnnCssInclude ID="DnnCssIncludeFontAwesome" runat="server" FilePath="https://use.fontawesome.com/releases/v5.0.13/css/all.css" Priority="110" />

it let me delete the whole folder and watch it get recreated. :-)

@david-poindexter
Copy link
Member

david-poindexter commented Jul 13, 2018

@john-Graham while it is outside the scope of this thread, we really don't recommend using the Client Dependency Framework (CDF) in DNN to load from a CDN. It technically works, but it does unfortunately have a side-effect of performance issues. Our recommendation is to pull your FA files local and let them be loaded from your server. Hopefully in the future, there will be enhancements made to the CDF in DNN to correct the performance issues.

Regarding Font Awesome 5, we would welcome any insight you have gained there. Let's create another issue so we can have some threaded conversation about that. Would you mind creating that issue so you can be easily mentioned in the thread?

Thanks!

@david-poindexter david-poindexter added this to the 2.0.0 milestone Jan 3, 2019
@david-poindexter david-poindexter changed the title should .gitignore ignore the whole dist folder? Update .gitignore to ignore dist folder Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants