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

extended httpImagePath to be compatible with downsampling if it ends with a / #34

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

Conversation

ebiggs
Copy link

@ebiggs ebiggs commented Sep 17, 2013

I needed to utilize httpImagePath but it wasn't compatible with the downsampling mechanisms since you could only specify one file. I maintained backwards compatibility by checking if the httpImagePath ends with a slash, if it does then append the file name, otherwise just use the httpImagePath

@ebiggs
Copy link
Author

ebiggs commented Sep 18, 2013

per the npm docs: do not compile coffeescript on install, or use install scripts in general. the man page recommends compiling coffee on prepublish, but checking the compiled coffee into git is better so that the repo can be used as a version in package.json.

G. Richard Bellamy added 2 commits September 21, 2013 21:11
@richardbutler
Copy link
Owner

@ebiggs this was the case originally, but I removed the JS from the repo as I was getting pull requests that only affected one or other of the CS and JS. Then, thinking about it, it feels a little dirty checking 'compiled' (read 'derived') code into Git, as you wouldn't do this for anything else. I'm torn.

Any ideas how other CS modules handle this issue?

@ebiggs
Copy link
Author

ebiggs commented Oct 18, 2013

It's tough. The install scripts seemed to work most of the time, but when I added node-spritesheet to a pretty complicated project it failed, and I couldn't figure out why. The description of it as an anti-pattern made me think it's something that's not worth fixing. My limited research showed that at least 1 respectable sizable CS module did the check-in-compiled-js solution. The problem is that this is something that can't really be automated and must be handled by developer guidelines, and then reviewed by you in a pull request. At the very least, perhaps minifying the js would help people realize that the source is coffee?

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.

None yet

2 participants