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

MatchOperator should be that type instead of int #1410

Merged

Conversation

ethervoid
Copy link
Contributor

Right now MatchOperator constants are ints and when you're trying to
assign them to a variable you get an int. If you try to use those
variables as an operator you get the following error:

cannot use operator (type int) as type MatchQueryOperator in argument to q.SetOperator

So, to fix this problem we just need to create MatchOperator constants

Right now `MatchOperator` constants are ints and when you're trying to
assign them to a variable you get an int. If you try to use those
variables as operator you get the following error:

```
cannot use operator (type int) as type MatchQueryOperator in argument to q.SetOperator

```

So, to fix this problem we just need to create `MatchOperator` constants
@mschoch
Copy link
Member

mschoch commented Jun 3, 2020

I'm fine with the change, but technically it does break the API since it changes the type of an exported identifier. It is inside a sub-package, so probably not as big a deal, but still seems like something that should be a 2.0 change, since it can be worked around anyway.

@ethervoid
Copy link
Contributor Author

@mschoch Yes, I thought that but didn't know if you're following semver. I agree with leaving that change to version 2.x given that is a breaking change. Hope it helps :)

@ethervoid
Copy link
Contributor Author

Read about release of version 2, would be these changes included?

@mschoch
Copy link
Member

mschoch commented Jan 6, 2021

I would be OK adding this to v2. It's relatively small, but since its a breaking change, this is a good opportunity. Thoughts @abhinavdangeti @sreekanth-cb?

@abhinavdangeti
Copy link
Member

Ya good time to merge I think.

@mschoch mschoch changed the base branch from master to fix-circular-deps January 6, 2021 17:39
@mschoch mschoch merged commit 8e14fc9 into blevesearch:fix-circular-deps Jan 6, 2021
@mschoch mschoch mentioned this pull request Jan 6, 2021
3 tasks
@mschoch
Copy link
Member

mschoch commented Jan 6, 2021

This has been merged to the 2.x branch, and I've updated the documentation to mention it. Thanks for the contribution and the reminder ping @ethervoid

@ethervoid
Copy link
Contributor Author

@mschoch thank you for taking care :)

mschoch added a commit that referenced this pull request Jan 12, 2021
* refactor to remove circular deps

* reduce index and segment API surface area

* update Advanced() method (#1509)

* relocate store pkg inside upsidedown (#1510)

* remove Advanced() method from scorch (#1511)

* remove use of dump methods from index reader (#1516)

* remove remaining blevex imports (#1520)

* update analysis.TokenFrequency to use options (#1522)

* switch default index to scorch with latest zap (#1528)

* MatchOperator should be that type instead of int (#1410)

Co-authored-by: Sreekanth Sivasankaran <sreekanth.sivasankaran@couchbase.com>
Co-authored-by: Mario de Frutos Dieguez <mario@defrutos.org>
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

3 participants