Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upProper .gitignore support #125
Comments
feross
added
the
feature request
label
May 13, 2015
This comment has been minimized.
This comment has been minimized.
tunnckoCore
commented
May 21, 2015
|
Why not just parse gitignore from cwd and then append defaults? And then update them like in the last lines of the index.js which is weird btw, but understandable. |
This comment has been minimized.
This comment has been minimized.
|
Part of the issue is the gitignore patterns are not the same as minimatch/glob patterns. The ignore package above looks looks like it reconciles these patterns. |
feross
added
the
help wanted
label
May 22, 2015
This comment has been minimized.
This comment has been minimized.
|
PR welcome! The |
This comment has been minimized.
This comment has been minimized.
|
If we do this, could it be done in a 4.0 release as couldn't it be stated that this could violate semver if someone needed to modify their |
This comment has been minimized.
This comment has been minimized.
junosuarez
commented
May 28, 2015
|
it should follow the npm ignore algorithm: https://docs.npmjs.com/misc/developers#keeping-files-out-of-your-package |
This comment has been minimized.
This comment has been minimized.
|
Okay, I have this working. Should it be 3.x or 4.0.0? @jprichardson The "ignore" property of package.json isn't going away. The .gitignore is purely additive; it's adding new ignored files. That makes me think this should be 3.x. No one's tests should break because of this. When I originally added .gitignore support, it was also done in a minor release. Anyone have any thoughts (in the next 30 minutes) before I cut this release? |
This comment has been minimized.
This comment has been minimized.
|
Well, actually, I have another definitely breaking change (operators go before line break ) I wanted to get into 4.0.0. Might as well just release this change together with that one to be on the safe side. |
feross
closed this
in
6476721
May 30, 2015
This comment has been minimized.
This comment has been minimized.
|
Okay, standard 4.0.0 is released! Enjoy :) |
This comment has been minimized.
This comment has been minimized.
Excellent :) |
This comment has been minimized.
This comment has been minimized.
junosuarez
commented
Jun 10, 2015
|
for the sake of anyone searching back through issues trying to figure out a change in ignore behavior like I was: in ~3.0.0, package.json |
This comment has been minimized.
This comment has been minimized.
|
@jden That's odd. |
This comment has been minimized.
This comment has been minimized.
junosuarez
commented
Jun 10, 2015
|
@feross on ci *= ~/dev/agilemd/api
2:21> node -p "require('os').platform() + ' ' + require('os').arch()"
win32 x64
ci *= ~/dev/agilemd/api
2:21> node -v
v0.10.36 |
feross
added a commit
that referenced
this issue
Jun 10, 2015
This comment has been minimized.
This comment has been minimized.
|
@jden I think I know what's causing this, but it's not easy for me to test on Windows. Can you try using the code on the |
feross
reopened this
Jun 10, 2015
This comment has been minimized.
This comment has been minimized.
junosuarez
commented
Jun 11, 2015
|
@feross I tried that out and get the same behavior. Note, I'm setting my ignores in the |
This comment has been minimized.
This comment has been minimized.
|
@jden Yep, I'm aware you're using the |
This comment has been minimized.
This comment has been minimized.
junosuarez
commented
Jun 11, 2015
|
@feross yep, confirmed working! |
This comment has been minimized.
This comment has been minimized.
|
@jden Cool, thanks for the help. I released this fix as 4.1.1. |
feross commentedApr 27, 2015
This looks like it actually does the trick: https://www.npmjs.com/package/ignore