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

Ignore when removing a non-existent document on the index #24

Merged
merged 2 commits into from
May 8, 2013
Merged

Ignore when removing a non-existent document on the index #24

merged 2 commits into from
May 8, 2013

Conversation

piranna
Copy link
Contributor

@piranna piranna commented Apr 30, 2013

No description provided.

@@ -51,6 +51,21 @@ test('removing a document from the index', function () {
equal(idx.documentStore.length, 0)
})

test('removing a non-existent document from the index', function () {
var idx = new lunr.Index,
doc = {id: 1, body: 'this is a test'}
Copy link
Owner

Choose a reason for hiding this comment

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

You've missed out a comma here, so doc2 is a global.

Other than that this change looks good, if you add the missing comma then I can merge this in and create a new release, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll take a look later :-)
El 07/05/2013 10:51, "Oliver Nightingale" [email protected]
escribió:

In test/index_test.js:

@@ -51,6 +51,21 @@ test('removing a document from the index', function () {
equal(idx.documentStore.length, 0)
})

+test('removing a non-existent document from the index', function () {

  • var idx = new lunr.Index,
  •  doc = {id: 1, body: 'this is a test'}
    

You've missed out a comma here, so doc2 is a global.

Other than that this change looks good, if you add the missing comma then
I can merge this in and create a new release, thanks.


Reply to this email directly or view it on GitHubhttps://github.com//pull/24/files#r4109226
.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, now you can fetch it :-) I'm not used to declare variables this way,
I use a var instance per variable... :-)

2013/5/7 [email protected] [email protected]

Ok, I'll take a look later :-)
El 07/05/2013 10:51, "Oliver Nightingale" [email protected]
escribió:

In test/index_test.js:

@@ -51,6 +51,21 @@ test('removing a document from the index', function () {
equal(idx.documentStore.length, 0)
})

+test('removing a non-existent document from the index', function () {

  • var idx = new lunr.Index,
  •  doc = {id: 1, body: 'this is a test'}
    

You've missed out a comma here, so doc2 is a global.

Other than that this change looks good, if you add the missing comma then
I can merge this in and create a new release, thanks.


Reply to this email directly or view it on GitHubhttps://github.com//pull/24/files#r4109226
.

"Si quieres viajar alrededor del mundo y ser invitado a hablar en un monton
de sitios diferentes, simplemente escribe un sistema operativo Unix."
– Linus Tordvals, creador del sistema operativo Linux

olivernn added a commit that referenced this pull request May 8, 2013
Ignore when removing a non-existent document on the index
@olivernn olivernn merged commit 0f6c519 into olivernn:master May 8, 2013
@olivernn
Copy link
Owner

olivernn commented May 8, 2013

This fix is now available in version 0.3.1, thanks for the patch!

@piranna
Copy link
Contributor Author

piranna commented May 8, 2013

Welcome :-)
El 08/05/2013 17:58, "Oliver Nightingale" [email protected]
escribió:

This fix is now available in version 0.3.1, thanks for the patch!


Reply to this email directly or view it on GitHubhttps://github.com//pull/24#issuecomment-17615215
.

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