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

browserify _extensions taken into account #4

Closed
wants to merge 5 commits into from

Conversation

nullpotent
Copy link

Fixes issue #3

@nullpotent
Copy link
Author

Entschuldigung, I have 0 experience with all of this.
This commit opened another issue as if it is a real one; which is strange to me, is this expected?
How should I commit next time around?

@nullpotent nullpotent closed this May 25, 2014
@joeybaker
Copy link
Owner

Thanks @iccthedral. Here's the way GH does this:

When you open a PR, it creates a new issue. That's intentional.

Your PR description should include something about what you did, and the comment "fixes #" for an issue. (Which you did!)

I've reopened this PR, and will take a look tonight. When I merge it, it'll close the issue for us and the PR.

@joeybaker joeybaker reopened this May 27, 2014
, Browserify = function(opts){
var self = this;
self._extensions = ['.js', '.json']
.concat(opts ? opts.extensions: []).filter(Boolean)
Copy link
Owner

Choose a reason for hiding this comment

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

Please run npm i -g jscs; jscs . There are several style errors that should be corrected.

Copy link
Author

Choose a reason for hiding this comment

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

I remember I've ran this a month ago and there were no style errors whatsoever. I've ran it again today and there are 36 some really nonsensical (to my untrained eye) errors. Here's the output

@joeybaker joeybaker mentioned this pull request Jun 11, 2014
@JohnRandom
Copy link

Hey guys, how is this coming along? Can I do anything to help? I could really use the patch right about now :)

@joeybaker
Copy link
Owner

@iccthedral any thoughts on my feedback?

@nullpotent
Copy link
Author

@joeybaker Yes, but can you guys wait for just one more day. I'm on vacation atm.

@JohnRandom
Copy link

Any progress on this issue? I'm still willing to help out..

@nullpotent
Copy link
Author

@JohnRandom Sure, do you understand the problem I'm having?
@joeybaker's comments are all spot on except I think on remapify:patterns event... so, could you please comment on my PR as well? Maybe I did something wrong.

@joeybaker joeybaker closed this in 386d45f Aug 9, 2014
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