-
Notifications
You must be signed in to change notification settings - Fork 25
Fix-#363 'type should be enum' #375
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
Conversation
brunoocasali
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Dishant10, thanks for your PR!
The CI must pass in order to merge your contribution! Check the linter messages and make sure your tests are passing locally :)
|
@brunoocasali can you please guide me what do I have to do in order to fix the other two failing tests. |
|
Any updates ? @brunoocasali |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments they should help you fix the remaining issues + get this PR approved :)
|
|
||
| /// Type of the task. | ||
| public let type: String | ||
| //public let type: String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should remove this comment.
| public let type: String | ||
| //public let type: String | ||
| public enum Type:Codable { | ||
| case indexCreation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation is not following the guidelines of the project. Also, you can move this enum to the bottom of the file but still inside of the scope of the public struct Task.
Type is a protected word in Swift, so you can use TaskType instead.
| case dumpCreation | ||
| case taskCancelation | ||
| case taskDeletion | ||
| case snapshotCreation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /// Type of the task. | ||
| public let type: String | ||
| //public let type: String | ||
| public enum Type:Codable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Thank you! @brunoocasali this will help me a lot! |
|
Hii @brunoocasali I'm sorry I was away for some time. I've made some changes, how can I test and get the messages on my local before raising a PR. ? |
|
I also had some doubts while fixing typing issues as you mentioned in the IndexesResults.swift file. So I want to test it locally first. |
Hi @Dishant10 good to see you here! You can follow the instructions written here: |
|
Closing due to inactivity! |


Pull Request
Related issue
Fixes #363
What does this PR do?
Makes the type variable an enum and all the cases are made as per the issue description.
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!