-
Notifications
You must be signed in to change notification settings - Fork 628
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
Removed all code related to sign in through google from app. #3333
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.
@ahsanishaq721 fix merge conflicts and add "Closes #ISSUE_NUMBER" to the PR description
Added close in PR description but not understanding the merging conflicts. Can you help? What i have to do |
@ILIYANGERMANOV done with resolved conflicts. |
@ahsanishaq721 now you need to fix the PR checks. Detekt is failing. Use the guide from the description |
@ahsanishaq721 run gradle task for detekt in local and fix the issues until build is successful. For windows you can run |
Thank u @suyash01 |
@ILIYANGERMANOV I am facing gradle issues, can I do some fixes in gradle in same branch and should i make another PR? |
|
There should be detekt lint error message above the build failed error message. It is not highlighted red. |
@suyash01 what should I do now? now detekt will be passed? |
@ahsanishaq721 read the CI Troubleshooting guide. Click "Details" on the failed detekt check and you'll see Property 'style>OptionalWhenBraces' is deprecated. Same functionality is implemented in BracesOnWhenStatements. |
There are unused imports and fields |
Ok, but i didn't import or add anything new in code. I have just removed the google sign in related code, which was my task. |
@ILIYANGERMANOV creating PR again |
@ILIYANGERMANOV I have done previously mentioned tasks mentioned by detekt. Now it is showing a longer list. These are existing issues and not related to my task I think. |
You can either:
Read the Troubleshooting guide |
i have run ./gradlew detektBaseline command but it failed the build process. |
@ahsanishaq721 can you trying using |
@ILIYANGERMANOV At last all checks have been passed. I was first time working on detekt so sorry for delay. |
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 job fixing the CI 👏 Found some issues that we need to fix before merging
@ILIYANGERMANOV I have done changes which you have requested. Please check. The checks are in progress. |
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.
LGTM!
Now, lint have failed. It happened for the first time. Why? |
Retrying it, seems like a GitHub Actions problem |
Ok done. Nice to work with you. It was my first small contribution to any open source project. |
…ts and files.
Pull request (PR) checklist
Please check if your pull request fulfills the following requirements:
main
branch.Screen recording of interacting with your changes:
What's changed?
Describe with a few bullets what's new:
...
...
Risk factors
What may go wrong if we merge your PR?
In what cases won't your code work?
Does this PR close any GitHub issues?
Troubleshooting CI failures ❌
Pull request checks failing? Read our CI Troubleshooting guide.