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

NodeJS support - npm package - tests #4

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

NodeJS support - npm package - tests #4

wants to merge 2 commits into from

Conversation

it-ony
Copy link

@it-ony it-ony commented May 8, 2013

Hi,

I added nodejs support for your library and prepared it for publishing to npm. Just go into the directory and use npm publish. See https://npmjs.org/doc/publish.html.

I also added two simple tests with mocha. Use mocha -R spec inside the project directory to run the tests. The project is also prepared to to run the tests on http://travis-ci.org.

@vkiryukhin
Copy link
Owner

Hi it-ony.

Thank you for your participation.
In fact, vkBeautify already has nodejs version which is known as "pretty-data"

https://github.com/vkiryukhin/pretty-data
https://npmjs.org/package/pretty-data

Does your changes have any advantage over existing pretty-data version?
Thank you,

--Vadim

@it-ony
Copy link
Author

it-ony commented May 10, 2013

Hi,

I saw the pretty-data version, after I changed the vkBeautify repository.

I think having one library that work for nodejs as well as for the browser should be generally the goal. This is in my opinion the biggest advantage over the pretty-data version.

This is possible from scratch because your library has no dependencies. This can be archived using the following snipped.

(function (exports) {

    exports.VkBeautify = VkBeautify;
    exports.vkbeautify = new VkBeautify();

})(typeof(exports) === "undefined" ? this : exports);

// in a browser this will be the window object

I think another advantages are the way the tests are written. No just printing the result, but also assert against the expected result. Modifying the tests to this approach, makes it possible to run them on a continues integration server.

It's up to you, but I promise the open source community will thank you for having one, well tested library.

-- Tony

return text.replace(/\s{1,}/g, " ").replace(/\s{1,}\(/, "(").replace(/\s{1,}\)/, ")");
};

exports.VkBeautify = VkBeautify;
Copy link
Author

Choose a reason for hiding this comment

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

Support for nodejs and browser at the same time.

@xaka
Copy link

xaka commented Mar 31, 2014

+1, i'm looking for a npm and bower packages of vkBeautify as well. It's the matter of npm publish and bower publish after all, plus you can automate it with Travis CI if you don't want to waste time doing it manually.

@danielmcq
Copy link

+1 for merging this pull request.

@Gerst20051
Copy link

👍

the-daria pushed a commit to the-daria/vkBeautify that referenced this pull request Apr 9, 2018
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.

5 participants