search

YUICompressor-NET / YUICompressor.NET

Port of Yahoo!'s Java YUICompressor to .NET
BSD 3-Clause "New" or "Revised" License
122 stars 61 forks source link

prepend NewLine to separate merged files #11

Closed katlimruiz closed 8 years ago

katlimruiz commented 9 years ago

This is to avoid syntax errors Example: jquery final line //# sourceMappingURL=jquery.min.map

invalidates the first line of bootstrap /*! * Bootstrap v3.3.5

when both are in the same line. That is why we need to separate both with a new line. This only happens when merging two files. If it's only one, do not add it. Tests were updated as well.

freeranger commented 9 years ago

I'd like to see a test for this new functionality that confirms that multiple files are joined with a separating newline between them. You have updated the existing tests so they still work but have not added any new tests

katlimruiz commented 8 years ago

I;m not sure why it fails, compressednewline.js exists in the repository, exists in csproj (test), is configured as Copy Always. Even line 259 in AppVeyor, says: Copying file from "C:\projects\yuicompressor-net\Code\Yahoo.Yui.Compressor.Tests\Javascript Files\compressednewline.js" to "bin\Debug\Javascript Files\compressednewline.js"

freeranger commented 8 years ago

Can you make the test name more descriptive please, describing the intent. The test name should read like english and describe what should happen. For example:

Your test name is "Verify_MinifiedNoCompression_NewLine". What does this mean? What are you verifying?
"When_The_CompresisonType_Is_NoneThen....[Something_Should_Happen]"

Also would like to see a test with uncompressed files as well as pre-compressed files and since your change is explicitly adding a newline between files, I think you should actually explictly test that a newline is inserted.

Finally, if it doesn't build then it can't come in - we can't accept a broken pull request!

freeranger commented 8 years ago

Closing as the author has not been back in touch