Skip to content

Add Makefile#21

Closed
dorako321 wants to merge 10 commits intoaadsm:masterfrom
dorako321:master
Closed

Add Makefile#21
dorako321 wants to merge 10 commits intoaadsm:masterfrom
dorako321:master

Conversation

@dorako321
Copy link

This Makefile can make uncompressed file (for developer) and compressed file. Moreover, It can remove files in dist directory by clean command.

"make all" this command outputs uncompressed/compressed file in dist directory
"make clean" this command removes files in dist directory

Makefile Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use “?=” instead of “=”? That way I can use the closure compiler I want without changing the makefile and getting a dirty tree.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i fixed it. thanks

@filcab
Copy link

filcab commented Oct 8, 2013

Hi dorako321,

I've looked at your makefile but it seems to hard-code too much stuff. Could you take a look a this suggestion?
This way, I find it becomes easier to move files around and add files to the library. The rules to generate the files also seem more explicit, to me.

Thank you,

Filipe

# output dir
DIR = dist
PRODUCTS = id3.min.js id3.dev.js
OUTPUTS = $(PRODUCTS:%=$(DIR)/%)
LIBS = id3.lib.js id3.core.js
CLOSURE_COMPILER ?= /usr/local/closure-compiler/compiler.jar

.PHONY: all clean
all: $(OUTPUTS)
# The lib and core files are intermediates, not needed after compilation
.INTERMEDIATE: $(LIBS)

# Search for JS files in src/
vpath %.js src

# Actual dependencies for each lib
$(DIR)/id3.lib.js: stringutils.js bufferedbinaryajax.js filereader.js base64.js
$(DIR)/id3.core.js: id3.js id3v1.js id3v2.js id3v2frames.js id4.js
$(DIR)/id3.dev.js: $(LIBS:%=$(DIR)/%)

$(DIR)/%.js:
    cat $^ > $@

$(DIR)/id3.min.js: $(DIR)/id3.dev.js
    java -jar $(CLOSURE_COMPILER) --compilation_level ADVANCED_OPTIMIZATIONS \
        --js $< > $@

clean:
    rm -f dist/*

@dorako321
Copy link
Author

Hi filcab.
thanks your opinion. i fixed it.

.project Outdated
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this file from the PR please? :-)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i removed it. thx

@aadsm
Copy link
Owner

aadsm commented Oct 14, 2013

Can you please rebase your 6 commits into a single one?
There's no need to keep all the fix up history.

@dorako321
Copy link
Author

is this correct?
please correct it.

@aadsm
Copy link
Owner

aadsm commented Oct 27, 2013

Hi @dorako321,

I'm really thankful for all the work you put on this but I've just noticed that make is not available on a standard installation of Windows or MacOSX, you need to install specific developer tools.
I'm sorry, but because of this I prefer to keep the shell script that will work on a standard MacOSX installation. I also prefer to avoid having to maintain two tools to create the minimized version.

To preserve your work and credit you I've created a branch with the Makefile: https://github.com/aadsm/JavaScript-ID3-Reader/tree/makefile

@aadsm aadsm closed this Oct 27, 2013
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.

3 participants