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.

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

build,refactor: fix build.gradle files with new repos added, refactor… #94

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

leoossa
Copy link

@leoossa leoossa commented Dec 24, 2022

… and remove obsolete dependency for hashing, use native method instead

Please kindly ask you to merge

… and remove obsolete dependency for hashing, use native method instead
Copy link

@pixincreate pixincreate left a comment

Choose a reason for hiding this comment

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

jcenter() is deprecated and we should move to use mavenCentral(). I tried to remove directly, but it has certain dependencies in the code that resulted for the build to fail but did caused no harm when I removed jcenter() from project's build.gradle
Also, I suggest you to update the gradle to 7.3.1

@@ -116,8 +116,7 @@ dependencies {
implementation 'com.mikhaellopez:circularprogressbar:3.0.3'
implementation 'org.jsoup:jsoup:1.14.1'
// https://gitlab.com/eyeo/distpartners/libadblockplus-android/-/tags
implementation 'org.adblockplus:adblock-android-webview:5.0.1'

Choose a reason for hiding this comment

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

What's the need to downgrade?

Copy link
Author

Choose a reason for hiding this comment

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

Given version was not resolving properly (I tried to investigate it but with no result). That version builds fine

Choose a reason for hiding this comment

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

This caused issues with AdBlock feature or functioning of the app overall?

Copy link
Author

Choose a reason for hiding this comment

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

Gradle couldn't find this version and download it

Choose a reason for hiding this comment

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

Android Studio suggessted me to update to 7.3.1 along with other libraries

Copy link
Author

Choose a reason for hiding this comment

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

Does it build successfully for you with 5.0.1?

Copy link

@pixincreate pixincreate Dec 25, 2022

Choose a reason for hiding this comment

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

Understandable. The app uses many old libraries and functionalities that easily breaks upon upgrading. I got to know about these by the logs in CI/CD.
Yes, I was able to build it directly. I did even upgrade the things. I cherry picked your commit yesterday to my repo and they'll eventually got downgraded. Haven't checked it yet due to lack of time.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link

@pixincreate pixincreate Dec 25, 2022

Choose a reason for hiding this comment

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

I just tried to build and yes, yes it failed now. I wonder why and how did that failed as I was able to build it yesterday as well as day before yesterday.

maybe it's due to repo havingt only 4.4.0 listed

This IMO might be one of the possible reason. I still personally prefer uBlock over adblock tho.

Copy link
Author

Choose a reason for hiding this comment

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

Me too. But I just wanted to hot fix stuff in order to build the repo. There was no reasoning behind decisions I made apart from successful build. You can change it however you like, though.

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.

2 participants