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

Moving npm package upstream #192

Merged
merged 85 commits into from
Mar 19, 2013
Merged

Conversation

evocateur
Copy link
Contributor

As convenient as git and GitHub make maintaining a very lightweight fork (virtually no modifications to existing code), I think it's time to fold my npm package back into the core.

Since the project is a Python/JS hybrid, of sorts, I did not follow the conventional npm source organization (bin and lib dirs, etc). These are things we can decide later, if desired (they're not required, just convention).

Lamentably, I have not written tests for the CLI interface I wrote (though I have exhaustively tested it in practice). This is largely because of the difficulty of integrating the existing functional tests with a convenient npm-based test framework like Mocha or Vows. I have experience with both frameworks, and am willing to contribute framework-based tests in the future.

Following the npm convention, the existing tests can be run via npm test in the project root. (Controlled by the scripts property of package.json)

Once merged, we'll need to modify the repository.url and possibly the contributors list (I just pulled what I could find from index.html and the README. As long as we're still using the js-beautify package identifier for npm, I can add @einars and @bitwiseman as owners (after you create accounts via npm's adduser subcommand), which would allow you to publish new versions to the npm registry.

I would be happy to effect these changes in the core repo directly, if you're willing to give me contributor access. Otherwise, I'll just make another pull request (adding ownership regardless).

Daniel Stockman added 30 commits July 13, 2012 16:52
Daniel Stockman added 25 commits March 13, 2013 13:06
…rs from the configured type, do the right thing. closes #2 (for real)
@evocateur evocateur mentioned this pull request Mar 19, 2013
@evocateur
Copy link
Contributor Author

Also, I am totally open to rebasing all of these changes into one gigantic commit. It would be a shame to lose the history, though.

@bitwiseman
Copy link
Member

I don't see any reason to squash them.

Your changes are basically orthogonal to the scripts and LGTM. I see no reason not to merge and if @einars has any feedback we can make further changes.

bitwiseman added a commit that referenced this pull request Mar 19, 2013
@bitwiseman bitwiseman merged commit f6757d0 into beautifier:master Mar 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants