docs: hammerjs using systemjs.config #10653
Conversation
googlebot
added
the
cla: yes
| } | ||
| 'hammerjs': 'npm:hammerjs', | ||
| }, | ||
| packages: { |
There was a problem hiding this comment.
The indentation here is inconsistent with the rest of the code snippet.
guides/getting-started.md
Outdated
| 'hammerjs': 'npm:hammerjs', | ||
| }, | ||
| packages: { | ||
| hammerjs: { main: './hammer.js', defaultExtension: 'js'} |
There was a problem hiding this comment.
You've got an extra space before and after the curly brace. Also shouldn't this use the minified version of the file?
There was a problem hiding this comment.
systemjs, normally used as development. And if someone wants to bring it in production, I will assume they should know how to config. it.
But if @crisbeto believes it is better to have minified version, please let me know.
There was a problem hiding this comment.
SystemJS does have a bundler that can be used for production, but I'm fine with continuing to use the non-minified version. The comment from above about the indentation still stands though.
There was a problem hiding this comment.
@crisbeto somehow, I cannot push my changes from my fork anymore. Do you want to take over this pull and we can have job done?
https://github.com/Longfld/material2/blob/master/guides/getting-started.md
For minified version:
hammerjs: {main: './hammer.min.js',defaultExtension: 'js'}
There was a problem hiding this comment.
That makes it trickier with the CLA. We're not in a rush to get this one in anyway. You might be getting blocked from pushing to your fork if your git email is out of sync with the one from GitHub.
There was a problem hiding this comment.
@crisbeto thanks enlighten me! I can push from my fork now. waiting for your further comments
There was a problem hiding this comment.
Sorry for being nitpicky, but it looks like the indentation still doesn't line up with the rest of the file.
There was a problem hiding this comment.
You are right, it is me, got difficult to commit in, please see #10711
josephperrott
changed the title
|
Replace by #10711 |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
To re-do and replace #7918