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

chore: introduce example and lib modules #36

Merged
merged 3 commits into from
Jun 17, 2022

Conversation

wzieba
Copy link
Collaborator

@wzieba wzieba commented Jun 13, 2022

+ remove unused files.

Closes: #35

Description

This PR creates two modules: parsely and example so the structure of the project is:

Root project 'parsely-android'
+--- Project ':example'
\--- Project ':parsely'

example module contains a sample app and parsely - the SDK, which will be offered on MavenCentral.


I've also removed a few lines of unused code in this PR but not all of it - I'll handle it in a separate PR.

@ParaskP7 ParaskP7 self-assigned this Jun 17, 2022
@ParaskP7 ParaskP7 self-requested a review June 17, 2022 11:04
Copy link
Collaborator

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

👋 @wzieba !

Really nice work on restructuring the project so that it builds from the root, right after someones clones it, and also removing the duplicate com.parsely.parselyandroid files, effectively making it a library instead.

I cloned the project and made it work with the old structure, I then pointed to this PR's branch and did the same with the new structure. Everything worked as expected. Thanks also for cleaning up the unnecessary testing related structure, some build configuration and a few of the IDE files that should be committed to the codebase anyway.

Everything looks so much better now and works as expected. It is a 👍 from my side!


  • My testing mainly involved running clean, assembleDebug and adb install to install the app. Everything worked as expected and with no issues. 💯
  • In addition to that, I also tryied to run assembleRelease but as you would expect as well I got a failure due to the targetSdkVersion 28 that is no longer support. Which, to be honest is why you are doing all this work anyway. Thus, I am sure you are aware of it. 🥇
  • I would have added other minor comments related to for example ProGuard, extra Gradle configuration, etc, but at this point I too think it is just better to progress anyway and do an overall clean-up/re-organization at the very end and in case we need to, that is after this project is updated to the point we feel comfortable with it. 👍

@wzieba
Copy link
Collaborator Author

wzieba commented Jun 17, 2022

Thanks a lot for the review @ParaskP7 🙏 !

I would have added other minor comments related to for example ProGuard, extra Gradle configuration

Some of the extra Gradle configurations should be probably resolved in the next PRs but I agree that we can do overall clean-up at the very end 👍 .

@wzieba wzieba merged commit 61393b4 into milestone-1/maven_central_repository Jun 17, 2022
@wzieba wzieba deleted the example_module branch June 17, 2022 13:00
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.

None yet

2 participants