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

Remove JS and CSS assets from the repository #1581

Closed
ujifgc opened this issue Feb 11, 2014 · 11 comments
Closed

Remove JS and CSS assets from the repository #1581

ujifgc opened this issue Feb 11, 2014 · 11 comments
Milestone

Comments

@ujifgc
Copy link
Member

ujifgc commented Feb 11, 2014

@padrino/core-members #1579 is a hot mess +4,759 −8,510. We really should not have copies of the standard assets in the repository. What would be the most appropriate way to handle this?

@nesquena nesquena added this to the 0.12.1 milestone Feb 11, 2014
@Ortuna
Copy link
Member

Ortuna commented Feb 11, 2014

They should be their own gems. e.g. there is jquery-rails, fontawesome-rails etc. I know those gems are for end-users, but no reason we couldn't use the same convention. But I think a light framework to load external assets is also a good idea?

@skade
Copy link
Contributor

skade commented Feb 11, 2014

The problem is that those gems are meant for use with an asset pipeline. As we don't have one by default, we have to vendor or build specific integrations for those gems.

@ujifgc
Copy link
Member Author

ujifgc commented Feb 16, 2014

@padrino/core-members what do you think of using googleapis for jquery and bootstrapcdn for bootstrap and fontawesome? This does not require any pipelines and the only drawback is the client has to be online. And I think it should not be a problem if we write a task, say, assets:download that would put all the assets to public and build helpers to be inserted instead of the default ones.

This way we could remove ~1.5 mb of useless data from the repository and free ourselves from the pain of +4,759 −8,510 update-commits.

I made this branch https://github.com/padrino/padrino-framework/tree/clean-admin to test the cdns out and I'm willing to write the tasks if this solution is found appropriate.

@skade
Copy link
Contributor

skade commented Feb 16, 2014

I object for multiple reasons:

  • Obvious problems for developers that have an offline usecase
  • A download task with helpers is basically a mini asset-pipeline with a mini-package-manager
  • I don't think we should rely on another third-party except package distribution systems.

We should find a proper solution for the asset pipeline problem and padrino-admin is a good software we can use for that.

This suggestion is basically sidestepping the problem.

The 1.5 MB of data are not useless (they are the assets required to run every version of the program, repeatably) and huge changes bring huge diffs. A possible fix would be include the libraries as submodules.

Regards,
Florian

On 16 Feb 2014, at 20:47, Igor Bochkariov notifications@github.com wrote:

@padrino/core-members what do you think of using googleapis for jquery and bootstrapcdn for bootstrap and fontawesome? This does not require any pipelines and the only drawback is the client has to be online. And I think it should not be a problem if we write a task, say, assets:download that would put all the assets to public and build helpers to be inserted instead of the default ones.

This way we could remove ~1.5 mb of useless data from the repository and free ourselves from the pain of +4,759 −8,510 update-commits.

I made this branch https://github.com/padrino/padrino-framework/tree/clean-admin to test the cdns out and I'm willing to write the tasks if this solution is found appropriate.


Reply to this email directly or view it on GitHub.

@skade
Copy link
Contributor

skade commented Feb 16, 2014

Could you please elaborate why you think they are "clearly" useless?

On 16 Feb 2014, at 21:35, Igor Bochkariov notifications@github.com wrote:

What about https://github.com/padrino/padrino-framework/tree/master/padrino-admin/lib/padrino-admin/generators/templates/assets/images/font ? These files are clearly useless. Do you think they can be hosted away?

@ujifgc
Copy link
Member Author

ujifgc commented Feb 17, 2014

It smells. I'm afraid it's gonna rot. I'm sorry if I'm not clear enough.

@skade
Copy link
Contributor

skade commented Feb 17, 2014

If you add a piece a of custom code the vendorizes a specific external version, it will rot just as much, + you have to maintain that code.

@ujifgc
Copy link
Member Author

ujifgc commented Feb 17, 2014

Now someone has to review around thirteen thousand lines of code every now and then. How's that for "maintain"?

@skade
Copy link
Contributor

skade commented Feb 17, 2014

That code would need to be reviewed in any case, either because the code is referenced remotely or included in the repos. It is a first class part of the project.

I am perfectly fine if we found a solution for the problem of including assets in padrino proper. But as stated above, I vote against a custom solution sidestepping the problem.

@ujifgc
Copy link
Member Author

ujifgc commented Feb 17, 2014

Changing a link to a version of a file on a trusted CDN does not require reviewing a cluster-ton of code.

So, if I'm getting you correctly, to solve the problem properly, we need a perfectly agnostic pipeline for padrino (#1028), then we need something similar to padrino-static to contain all the supercommits, and then we require it from padrino-admin and instruct our fine pipeline to use it? Can we maybe start with a passable micropipeline that uses a CDN?

@ujifgc ujifgc modified the milestones: 1.0.0, 0.12.1 Mar 10, 2014
@ujifgc
Copy link
Member Author

ujifgc commented May 4, 2016

Closing as not a good idea until having a passable asset pipeline.

@ujifgc ujifgc closed this as completed May 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants