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

add support for search by artist #1

Merged
merged 2 commits into from
Mar 28, 2011

Conversation

patrickkettner
Copy link
Contributor

real quick hack to add support searching by artist name. Its needed by a project im working on, and I would really like for it to be in the npm'ed version of this.

@garrettwilkin
Copy link
Owner

Hey Patrick,

This is looking pretty good! You actually showed me how easy it is to add other search types to module. Not too shabby. Thanks for taking the time to do so. There's only one line in your request that I think I will remove: console.dir(). I like to keep the code absent of log messages. I have to thank you for exposing me to console.dir(). I just googled that and it seems pretty useful!

@patrickkettner
Copy link
Contributor Author

Yikes! didnt notice I left that in there. I was reversing some of what you did before I realized you had the raw attribute available.

Cheers

@garrettwilkin garrettwilkin merged commit 73fabc9 into garrettwilkin:master Mar 28, 2011
@garrettwilkin
Copy link
Owner

I am testing your branch now. I thought that the package require call for Artist looked suspicious, and now I see that when I run "vows tests/*" it fails saying that it can't find the artist class. Did you add that file but not include it in the submit?

@garrettwilkin garrettwilkin reopened this Mar 28, 2011
@@ -7,6 +7,7 @@
require.paths.unshift(require('path').join(__dirname,'./util'));

var Album = require('album').Album;
var Artist = require('artist').Artist;
Copy link
Owner

Choose a reason for hiding this comment

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

Where is the Artist class definition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just pushed.

@garrettwilkin
Copy link
Owner

Just published as version 0.0.3 :) Enjoy and thanks for contributing!

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.

None yet

2 participants