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

Update README.md #1101

Closed
wants to merge 1 commit into from
Closed

Update README.md #1101

wants to merge 1 commit into from

Conversation

mobilekosmos
Copy link

Updated gradle instructions according to new Android changes. Also added Kotlin version of code.

Updated gradle instructions according to new Android changes. Also added Kotlin version of code.
@mtrezza
Copy link
Member

mtrezza commented Oct 10, 2021

@L3K0V should this be merged after 2.0.0 release, or is this info already true for the current version; and is this PR still correct given the changes in #1095?

@azlekov
Copy link
Contributor

azlekov commented Oct 10, 2021

I do not understand the purpose of the changes. Maybe @mobilekosmos can elaborate further?

@mobilekosmos
Copy link
Author

@L3K0V what exactly is not clear?

Copy link
Contributor

@azlekov azlekov left a comment

Choose a reason for hiding this comment

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

Everything looks nice just wonder about the dependencyResolutionManagement. But we are good to go! 👍

README.md Show resolved Hide resolved
@mtrezza
Copy link
Member

mtrezza commented Oct 13, 2021

If we are not sure then let's look further into it. We don't need to include an option that is unrelated to the example which we want to demonstrate. If we want to recommend an option that is not related to SDK but good practice, we can add a separate "Best Practice" section to the docs, as we have in Parse Server.

@mobilekosmos
Copy link
Author

mobilekosmos commented Oct 13, 2021

Since Gradle 7 you get an error if specifying allProjects{} inside the app's module build.gradle. Also Android Studio project templates now generates that block in the settings.gradle file. More infos here
This change is needed when upgrading to gradle 7 as in my latest PR which is under review. Don't know if this works usign older version of gradle.

@mtrezza
Copy link
Member

mtrezza commented Oct 13, 2021

Thanks for clarifying, so we can merge this after your PR where you upgrade gradle. Which PR is it?

I'm still not sure about FAIL_ON_PROJECT_REPOS, it sounds like something that is a custom setting, up to the developers to decide, or is that required to implement the Parse SDK?

@mobilekosmos
Copy link
Author

FAIL_ON_PROJECT_REPOS: If this mode is set, any repository declared directly in a project, either directly or via a plugin, will trigger a build error.

We want to declare the repositories for all project centraly, so to avoid modules adds repositories which are anyways already declared centrally this flag fits the purpose I think. If for some reasson a module needs an extra repository some day you can change that flag anytime.

https://docs.gradle.org/current/javadoc/org/gradle/api/initialization/resolve/RepositoriesMode.html

@mobilekosmos
Copy link
Author

The build output with that flag when adding a repository to a module.gradle would be following:

A problem occurred evaluating project ':bolts-tasks'.

Build was configured to prefer settings repositories over project repositories but repository 'Google' was added by build file 'bolts-tasks\build.gradle'

@mobilekosmos
Copy link
Author

PR #1122

@mtrezza
Copy link
Member

mtrezza commented Oct 13, 2021

So if a developer wants to use the Parse Android SDK in their project, is it mandatory for them to add this?

repositoriesMode.set(RepositoriesMode.FAIL_ON_PROJECT_REPOS)

@azlekov
Copy link
Contributor

azlekov commented Oct 13, 2021

So if a developer wants to use the Parse Android SDK in their project, is it mandatory for them to add this?

No this is just for the scope of this project. If developer use the Parse Android SDK as Gradle dependency in their project there's no additional configurations to be made.

@mtrezza
Copy link
Member

mtrezza commented Oct 13, 2021

Why do we add this line to the readme then, where we instruct a developer how to use the Parse Android SDK as dependency?

Would it make more sense to change it to this? So we keep it as simple and compact as possible.

Dependency

Add this in your root build.gradle file (not your module build.gradle file):

// gradle >=7
dependencyResolutionManagement {
  repositories {
    // ...
    mavenCentral()
    maven { url "https://jitpack.io" }
  }
}

// gradle <7
allprojects {
  repositories {
    // ...
    mavenCentral()
    maven { url "https://jitpack.io" }
  }
}

@mobilekosmos
Copy link
Author

mobilekosmos commented Oct 14, 2021

I got you now. Partially, you don't put dependencyResolutionManagement in the gradle file, I will update the PR.

@mobilekosmos
Copy link
Author

I will close this PR and do the changes in the PR #1122 which upgrades gradle.

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.

3 participants