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

Ferini/kotlin gradle dsl #811

Closed
wants to merge 20 commits into from

Conversation

ferinagy
Copy link

@ferinagy ferinagy commented Dec 17, 2019

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

  • I ran ./gradlew spotlessApply before submitting the PR
  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing

🔮 Next steps

📸 Screenshots / GIFs

@keyboardsurfer
Copy link
Collaborator

Hi, thanks for the PR. How is this different to #807?

@ferinagy
Copy link
Author

Hi, thanks for the PR. How is this different to #807?

Hi, I did this as part of experimenting with kotlin DSL on a separate branch and wanted a way to share the diff of what was done. It was not intended to be merged in this state, that's why it is only draft. In the hindsight, I probably could just create a PR in my forked repo.

I saw the other one already exists and it is mostly similar to this one. I took some different approach in some areas (eg. I kept separate shared_dependencies.gradle.kts and test_dependencies.gradle.kts and moved them to buildSrc as precompiled plugins), but I am not sure if that makes this one better suitable than the other.

If you want to check it out and think this might be better candidate to merge, I am happy to clean it up, otherwise feel free to close it.

@keyboardsurfer
Copy link
Collaborator

@pfmaggi FYI

@pfmaggi
Copy link
Collaborator

pfmaggi commented Dec 30, 2019

As @ghus-raba wrote, the main difference is having the precompiled plugins for the dependencies while #807 keep them together in a module.

Most of the other things seems small differences.
#807 is still using AGP 3.6 as there's an issue with 4.0 when running the tests.

@pfmaggi
Copy link
Collaborator

pfmaggi commented Jan 2, 2020

Thanks @ghus-raba for the PR, I'm updating #807 using some of the ideas you used here.

@codingjeremy codingjeremy changed the base branch from master to main September 29, 2020 19:10
@ferinagy ferinagy closed this Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants