Skip to content

Fixed comparison mismatch - #269

Merged
toxinu merged 1 commit into
open-craft:masterfrom
asadazam93:asad/prod-1294
Apr 3, 2020
Merged

Fixed comparison mismatch#269
toxinu merged 1 commit into
open-craft:masterfrom
asadazam93:asad/prod-1294

Conversation

@asadazam93

Copy link
Copy Markdown
Contributor

PROD-1294

Description

Added empty check on 'min_characters'

Sandbox
N/A

Reviewers

If you've been tagged for review, please check your corresponding box once you've given the 👍.

Post-review

  • Rebase and squash commits

@asadazam93

Copy link
Copy Markdown
Contributor Author

@lgp171188 can you please review this PR?

@lgp171188

Copy link
Copy Markdown
Contributor

@asadazam93, @xitij2000 prioritizes and schedules the PRs for review. So I am pinging him.

@fysheets

fysheets commented Apr 1, 2020

Copy link
Copy Markdown

Hello! Wanted to check in on a potential review here, also to sanity check if we need to have all tests green before a review can be scheduled?

@asadazam93

Copy link
Copy Markdown
Contributor Author

I noticed that the py27 were failing on previous PRs as well and they were merged anyway. So I am assuming there is an issue with the python2 tests.

@xitij2000

Copy link
Copy Markdown
Member

@asadazam93 @fysheets Incredibly sorry for the delay! This slipped under my radar.

Yes, there are issues with the python2 tests. They seem to be passing locally but failing in the CI. If you find a solution to it that would be lovely, but we can merge this if all tests pass locally, even if they don't here.

@toxinu

toxinu commented Apr 2, 2020

Copy link
Copy Markdown
Contributor

@asadazam93 Approved as trivial change. 👍

Can you just bump the patch number in the setup.py file?

@asadazam93

Copy link
Copy Markdown
Contributor Author

@toxinu can you also please merge this PR? I don't have access to merge it

@toxinu

toxinu commented Apr 3, 2020

Copy link
Copy Markdown
Contributor

@toxinu can you also please merge this PR? I don't have access to merge it

I was about to merge it after the version bump but I can do it by myself. Anyway, thanks for your contribution. 👍

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.

5 participants