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.

Sign up for GitHub

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

Jump to bottom

Fix some IDE reported issues #17405

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix some IDE reported issues #17405

wants to merge 4 commits into from
+15 −10

Conversation

MritunjayTiwari14
Copy link

@MritunjayTiwari14 MritunjayTiwari14 commented Nov 10, 2024 •
edited by lukstbit
Loading

Purpose / Description

To Fix Android Studio Warnings and make the experience smoother and bug free.

Fixes

Approach

Looking for the Warning each one by one and solving them

How Has This Been Tested?

All the Warning were removed

Checklist

Please, go through these checks before submitting the PR.

  • [YES ] You have a descriptive commit message with a short title (first line, max 50 chars).
  • [ NO, Because All the Coding is about Cleanup] You have commented your code, particularly in hard-to-understand areas
  • [Yes] You have performed a self-review of your own code
  • [NO UI Changes] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • [NO UI Changes] UI Changes: You have tested your change using the Google Accessibility Scanner

@MritunjayTiwari14
Cleaned the Warning in the code:
11a5b1c 
1.HTML(Updated the obsolote align to text-align)
2.Changed the exit() to exitProcess()
3.Added String for the required Data
@MritunjayTiwari14
Merge branch 'main' of https://github.com/ankidroid/Anki-Android
f3d32c1
@welcome Welcome
Copy link

welcome bot commented Nov 10, 2024

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

lukstbit
lukstbit requested changes Nov 10, 2024
View reviewed changes
Copy link
Member

@lukstbit lukstbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing, I've left some issues below to fix.

Also you may want to read our git related wiki on how to contribute.

.codecov.yml Outdated
@@ -13,7 +13,7 @@ coverage:
default:
# dropping slightly is ok - this stops the check failing on test timing variability
# .12 was observed as the largest variability owing to timing forcing different test branch choice
threshold: 0.15
threshold: "0.15"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a not an issue and should be reverted.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay Sir

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't mark review requests as resolved if you didn't resolved them. So again, please revert this change.

README.md Outdated
@@ -41,15 +41,15 @@ Install
<div style="display:flex;">

<a href="https://play.google.com/store/apps/details?id=com.ichi2.anki&utm_source=global_co&utm_medium=prtnr&utm_content=Mar2515&utm_campaign=PartBadge&pcampaignid=MKT-Other-global-all-co-prtnr-py-PartBadge-Mar2515-1">
<img alt="Get it on Google Play" height="80"
<img alt="Get it on Google Play" height="250"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although the 80 value raises an issue, using other values makes those images too big. Instead of changing the value these issues should be suppressed: right click -> Show context actions -> Click the three dots beside "Set height value to 250" -> Suppress for tag

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still active, this also needs to be done for the other two values in this file.

README.md
@@ -1,5 +1,5 @@
<p align="center">
<img alt="" src="docs/graphics/logos/banner_readme.png"/>
<p style="text-align: center;">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the obsolete align attribute. Also revert the spacing change on the inner img tag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request above still active.

build.gradle.kts
}

val ciBuild by extra(System.getenv("CI") == "true") // works for Travis CI or Github Actions
// allows for -Dpre-dex=false to be set
// allows for -Dare-dex=false to be set
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this, it changes the meaning of the comment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay Sir

@lukstbit lukstbit changed the title My feature branch Fix some IDE reported issues Nov 10, 2024
@lukstbit lukstbit added Needs Author Reply Waiting for a reply from the original author squash-merge The pull request currently requires maintainers to "Squash Merge" labels Nov 10, 2024
@MritunjayTiwari14 MritunjayTiwari14 mentioned this pull request Nov 15, 2024
Closed
5 tasks
lukstbit
lukstbit requested changes Nov 20, 2024
View reviewed changes
.codecov.yml Outdated
@@ -13,7 +13,7 @@ coverage:
default:
# dropping slightly is ok - this stops the check failing on test timing variability
# .12 was observed as the largest variability owing to timing forcing different test branch choice
threshold: 0.15
threshold: "0.15"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't mark review requests as resolved if you didn't resolved them. So again, please revert this change.

README.md
@@ -1,5 +1,5 @@
<p align="center">
<img alt="" src="docs/graphics/logos/banner_readme.png"/>
<p style="text-align: center;">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request above still active.

README.md Outdated
@@ -41,15 +41,15 @@ Install
<div style="display:flex;">

<a href="https://play.google.com/store/apps/details?id=com.ichi2.anki&utm_source=global_co&utm_medium=prtnr&utm_content=Mar2515&utm_campaign=PartBadge&pcampaignid=MKT-Other-global-all-co-prtnr-py-PartBadge-Mar2515-1">
<img alt="Get it on Google Play" height="80"
<img alt="Get it on Google Play" height="250"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still active, this also needs to be done for the other two values in this file.

build.gradle.kts Outdated

// Top-level build file where you can add configuration options common to all sub-projects/modules.
// Top-level Build file where you can add configuration options common to all sub-projects/modules.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukstbit lukstbit lukstbit requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
Needs Author Reply Waiting for a reply from the original author New contributor squash-merge The pull request currently requires maintainers to "Squash Merge"
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MritunjayTiwari14 @lukstbit