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

Document Memory & Solr Adapters. #105

Merged
merged 1 commit into from Jul 10, 2017
Merged

Document Memory & Solr Adapters. #105

merged 1 commit into from Jul 10, 2017

Conversation

tpendragon
Copy link
Collaborator

Want to get this in one step at a time so that we're all good with everything before I do too much work and have to back track.

def initialize(object)
@object = object
end

# @return The solr document ID
Copy link
Member

Choose a reason for hiding this comment

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

[String]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

class SolrRow
attr_reader :key, :fields, :values
# @param key [Symbol] Solr key.
# @param fields [Array<Symbol>] Field suffixes to index into.
# @param values [Array<>] Values to index into the given fields.
Copy link
Member

Choose a reason for hiding this comment

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

Just Array or provide the inner type here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

def initialize(key, value, scope = [])
@key = key
@value = value
@scope = scope
end
end

##
# Represents a row in the solr document, used for isolating logic around
Copy link
Member

Choose a reason for hiding this comment

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

Solr doesn't have rows, so this seems forced, but it might require a class name change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. Changed the comment a little - regarding the name change, if you can think of a better name put in a ticket? It's internal, but naming's important.

@jcoyne jcoyne merged commit ce8ba3d into master Jul 10, 2017
@jcoyne jcoyne deleted the documentation branch July 10, 2017 19:10
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