-
-
Notifications
You must be signed in to change notification settings - Fork 270
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
Added "fun" to various Kotlin interfaces and updates SDKs a bit #104
Conversation
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.
Nice work! Thanks
Can you please add the ticket:
https://github.com/CanHub/Android-Image-Cropper/issues/102
To the Changelog file?
Btw, the CI is broken =/
On a side not, should we change the sample code so we are sure this doesn't break again?
CropImageView.CropShape.RECTANGLE_VERTICAL_ONLY -> binding.cropShape.chipRectangleVerticalOnly.isChecked = true | ||
CropImageView.CropShape.RECTANGLE_HORIZONTAL_ONLY -> binding.cropShape.chipRectangleHorizontalOnly.isChecked = true | ||
CropImageView.CropShape.RECTANGLE_VERTICAL_ONLY -> binding.cropShape.chipRectangleVerticalOnly.isChecked = | ||
true | ||
CropImageView.CropShape.RECTANGLE_HORIZONTAL_ONLY -> binding.cropShape.chipRectangleHorizontalOnly.isChecked = | ||
true |
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.
This changes broke the ktlintCI
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.
How so? Why did it break anything? What is the change here, anyway? I don't see any error here on the IDE... The sample works fine. Everything builds fine...
I think it only formatted this way for some reason. The code seems the same.
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.
Anyway, I've made it have a new line as the logs that you've shown say. Still a very weird issue.
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 error message:
> Task :sample:ktlintMainSourceSetCheck FAILED
/home/runner/work/Android-Image-Cropper/Android-Image-Cropper/sample/src/main/java/com/canhub/cropper/sample/options_dialog/app/SOptionsDialogBottomSheet.kt:191:62 Missing newline after "->"
/home/runner/work/Android-Image-Cropper/Android-Image-Cropper/sample/src/main/java/com/canhub/cropper/sample/options_dialog/app/SOptionsDialogBottomSheet.kt:193:64 Missing newline after "->"
The Ci link: https://github.com/CanHub/Android-Image-Cropper/pull/104/checks?check_run_id=2239558660
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.
It shows it even though I've added a new line now?
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.
Now is ok, you can see on the bottom of the github PR page =D
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.
How did you perform this check? How come this issue even exists, while the IDE is completely fine with it?
… , even though the IDE doesn't show any sign of an error.
I didn't create anything in the changelog file. Should I ? |
Yes please, More into the CONTRIBUTING file |
OK I've updated it. Hope it's how it's supposed to be. |
Now what will happen? |
I'm so confused. What's going on? |
I approved, was waiting CI to run, have a meeting in my work, merge it, create the release, JitPack build the package and make it available. I believe should be ready for usage =D |
I had a lot of emails. What is "CI", and how did you get the error before, while I didn't have any error on the IDE? |
CI means continuous integration. Is a serie of automated steps that keep our codeStyle. We use ktlint. Normally when projects use CodeStyle you can see it on the Another way to enforce this without commands, and the way most people use, select the whole file Later the CI will break to keep the code style in place, some times most people just throw in the CI and wait to see if fails, depends on the Dev |
So somehow the code style of the project was imported to the IDE, but the IDE failed to notice it, and didn't show a warning about it or format it as it wanted? |
It won't show any warning, sadly that's now hot it works =/. You need to run the commands, or the shorcuts or the terminal ones. |
But shouldn't it auto-format anyway to the rules that were set? |
It should when using the commands, if is not, maybe the AS is getting the wrong Style file |
ok |
close #102
Bug
Cause:
When refactoring from java we lost the ability of using lambda on our callbacks
How the bug was solved:
Add
fun
notation to the interfaces.