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

Index #37

Closed
wants to merge 17 commits into from
Closed

Index #37

wants to merge 17 commits into from

Conversation

averagehat
Copy link
Collaborator

Closes #29
This is working on my machine but I will add tests before it's ready for merging

  • fix how ID is set
  • add tests, inc. with multi-file/entry indexing
  • test/fix JSON file loading
  • try with big file
  • refactor so indexing lives outside of Seqr.handleCommand

@lianyi
Copy link
Collaborator

lianyi commented Sep 17, 2015

@averagehat Sounds good to you to merge this index branch? Bo is making FASTA indexer and after seeing the diff, It seems best to merge this branch first to avoid minimum code conflict.

@averagehat
Copy link
Collaborator Author

Yes let me see if I can get the build to pass or at least working on my machine. This branch may be a bit outdated from my local code.

Is he creating the fasta indexer as a Solr process/request handler? It would be great if he could make a pull request (or at least issue) even if he is not done, that would help with visibility and not duplicating work.

@lianyi
Copy link
Collaborator

lianyi commented Sep 17, 2015

@averagehat Gr8! Sounds a good plan!

@averagehat
Copy link
Collaborator Author

You/Bo may want to check out this in the meantime, I think it includes a fasta indexer #44

@averagehat
Copy link
Collaborator Author

The main purpose of this PR was to unify JSON and FASTA indexing. #45 has a different but working approach to that so I am going to close this and open up a new PR

@averagehat averagehat closed this Sep 22, 2015
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.

index command
2 participants