Skip to content
This repository has been archived by the owner on Jul 20, 2021. It is now read-only.

Css min #66

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Css min #66

wants to merge 3 commits into from

Conversation

robin-drexler
Copy link
Member

Currently saves about ~1.3kb.
But maybe it will save more in the future and having it should not do any harm.
Will probably also check for js minifying another time.

@ro-ka
Copy link
Member

ro-ka commented Jan 20, 2015

Ähm – node_modules got checked in?

@robin-drexler
Copy link
Member Author

Yes, makes deployment process much easier and more resilient against npm registry outages.
Are there any objections against checking in node modules?

@ro-ka
Copy link
Member

ro-ka commented Jan 20, 2015

I don't like that… Lots of bloat! Never had problems with an outage.

@robin-drexler
Copy link
Member Author

I personally don't mind repo size (if that's want you meant with Lots of bloat!).
This seems like a thing of personal preference to me, so I'd just keep it that way until actual problems arise, also because we'd have to adjust the deployment otherwise.

@robertkowalski
Copy link
Member

The outages of npm got better in the last year as they are using a cdn now. But that does not help if someone unplublishes a module what happens here and then:

Checking in the modules is still my preferred way for bigger projects, originating back when i was building npm together with isaac and domenic. npm still has checked in modules: https://github.com/npm/npm/tree/master/node_modules

beeing not able to deploy because one external dependency is down or even got unpublished on npm can get quite annoying and i am glad that you never experienced that.

i prefer to keep the external dependency count low to reduce the moving targets, currently we are relying on github for deployments already.

@ro-ka
Copy link
Member

ro-ka commented Jan 21, 2015

I don't mean the file size, it's just hard to review that… Github won't render the diffs. We rely on a bunch of different modules in various projects and never experienced an unpublishing.

@yfr
Copy link
Member

yfr commented Jan 21, 2015

I'm here on @ro-ka side.
This is what github says: Sorry, we could not display the entire diff because too many files (518) changed. So PR could not be checked really!
Never had any problems with getting node packages.

@robertkowalski
Copy link
Member

i don't see the problem here, this PR is splitted in two commits, one that adds the module and the other one adding the change

when i click on the commit that includes the actual change i don't have any rendering problems:

mmm

when i read https://docs.npmjs.com/misc/faq i see that npm now suggests officially

"If you are paranoid about depending on the npm ecosystem, you should run a private npm mirror or a private cache."

can you maintain a npm mirror for us @ro-ka @yfr? - i ask as i don't want to maintain that on my own

@ro-ka
Copy link
Member

ro-ka commented Jan 22, 2015

So, you are paranoid? 😉 I don't want to maintain one neither. Just marked that I never experienced problems with the public npm.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants