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

Search by Title, Id, Author, Description fields - enhanced UI and ready for merge #671

Merged
merged 19 commits into from
Dec 11, 2012

Conversation

TimLovellSmith
Copy link
Member

Fixes issues #659 and #668. Enhances Tag and Author links to do searches on relevant fields only.

Tim Lovell-Smith added 14 commits November 29, 2012 15:43
The first unit test is FAILing to find the word 'awesome' in the package description.
LuceneSearchService to use in-memory indexes during unit test runs.
…ization. Motive - trying to get all my new tests to pass.
But search such as 'id:NuGet assembly' results seem pretty random still.
…Ids work better and for Id phrase searches to work.
…arches with Tags: and Author: search. Also refine the way search strings are presented so that quote marks used in searches are less confusing. And revert some temporary debugging changes.
@TimLovellSmith
Copy link
Member Author

ping for review

{
public class LuceneSearchServiceFacts
{
private Directory _d;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we refactor these tests to avoid using an instance variable? xUnit does create a separate instance per-test, but I'd rather not depend on that. Maybe have the code that sets this use a Tuple or out-param to set it? I don't really like those either but they're clearer than a magic instance variable :).

Copy link

Choose a reason for hiding this comment

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

+1. Avoid instance variable if we can.

@TimLovellSmith
Copy link
Member Author

Thanks for all the feedback so far, it's helped! Who's up for round 2? :)

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.

2 participants