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

[Feature] [Fix] Cleaning static #184

Closed

Conversation

mfraezz
Copy link
Member

@mfraezz mfraezz commented Aug 20, 2015

Purpose

Closes #177
Closes #176
Closes #175

Changes

  • Put bootstrap css rules used by codepygments in default.css
  • Removed unnecessary static files in /pdb/, /pdf/, /svg/, and /tabular/
  • Remove reference to a deleted example file
    • It would never be used, MFR wouldn't get that far without a file being passed in:
var DEFAULT_URL = '[some research paper by Mozilla].pdf';
[...]
var file = 'file' in params ? params.file : DEFAULT_URL;

Side Effects

None, tested all filetypes that could be affected

CR Note

Commit 2 is exclusively deletions, view other commits for modifications

    -all 10 of them.
    -remove bootstrap dependency
    -There may be more, but these are not used
    -Example used iff file not given, MFR breaks before that
@mfraezz mfraezz changed the title [Feature] Cleaning static [Feature] [Fix] Cleaning static Aug 20, 2015
@lyndsysimon
Copy link
Member

Looks great. I've got some follow-up questions for you Steve, but they shouldn't be a part of this PR.

@lyndsysimon
Copy link
Member

Oh! I almost forgot the gold star: 🌟

@lyndsysimon lyndsysimon assigned sloria and unassigned lyndsysimon Aug 21, 2015
@icereval
Copy link
Contributor

I'm struggling to understand why we would want to customize third party static assets? Third party plugins are meant to be drop in upgrades to the latest version with the fewest number of changes possible to have them integrated w/ the MFR.

This seems like a lot of work to maintain this practice going forward and also adds significant risk of an asset being missed or mistakenly removed.

If we are concerned about bloat in the code base we might investigate fixing this issue with a dependency manager like Bower, thus third party libraries are only brought into the picture on build and can remain separate.

Reviewing the referenced tickets they can be addressed with minimal changes such as removing the references to dependencies.

Please resubmit w/o asset deletions.

@mfraezz
Copy link
Member Author

mfraezz commented Aug 24, 2015

I'll close this for now, get a PR up for the pygments issues and add an issue about using a dependency manager.

@mfraezz mfraezz closed this Aug 24, 2015
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