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

feat: setup project scheleton #49

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

Conversation

ilkeraslan
Copy link
Collaborator

  • Create shared module to manage shared logic via Kotlin Multiplatform.
  • Create android module for Android app.

@ilkeraslan ilkeraslan added the enhancement New feature or request label Feb 14, 2023
@ilkeraslan ilkeraslan changed the title feat/Setup Project Scheleton feat: setup project scheleton Feb 14, 2023
Copy link
Collaborator

@fardavide fardavide 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 your contribution!

I have a few notes about the changes, based on some concepts we agreed upon, and I would like to take the opportunity to improve the readme, to add guidelines, and facilitate future contributions, but also give a chance to speak out to those who didn't participate to the previous discussions! 🙂

Conventional commits

We agreed that every commit message should follow the conventional commits guidelines; for example, Add shared and android modules should be chore: add shared and android modules - note the prefix chore: and the de-capitalized sentence.

Running the tunes.sh scripts once should add the pre-commit hooks to your project so that you would receive an error with a message, in case the commits message doesn't match the defined pattern.

Build system (Gradle)

We agreed to use Groovy, over KScript (.kts), in order to keep the maximum of performance for Gradle.

In order to keep the modules' configuration accessible and coherent, we completely rely on our conventional plugins located into build-tools to configure our modules: for example, the iOS targets initialization should be done by the MultiplatformPlugin.

Compose and resources

We agreed to use Jetpack Compose, so in my opinion, we should not add various layouts and XMLs, including also the icons.

@ilkeraslan ilkeraslan force-pushed the feat/project_scheleton branch 5 times, most recently from 0958553 to ac16856 Compare February 20, 2023 18:16
@ilkeraslan ilkeraslan marked this pull request as ready for review February 20, 2023 22:22
Copy link
Collaborator

@fondesa fondesa left a comment

Choose a reason for hiding this comment

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

Maybe I have missed something but why do we need a project skeleton?
Shouldn't we start adding directly our features/real modules?

If the purpose of the skeleton is just creating a sample where we can try out things, I think the right location should be :playground.

@fardavide
Copy link
Collaborator

Maybe I have missed something but why do we need a project skeleton?
Shouldn't we start adding directly our features/real modules?

This is also a very valid point that I haven't been able the highlight

@ilkeraslan
Copy link
Collaborator Author

Maybe I have missed something but why do we need a project skeleton? Shouldn't we start adding directly our features/real modules?

If the purpose of the skeleton is just creating a sample where we can try out things, I think the right location should be :playground.

My goal was to have a starting point for the feature modules that we are still discussing. It came to my understanding that we haven't decided yet which are our main features, so in the meantime I see this as an opportunity to setup the base structure.

If the PR is not appropriate or if we know which feature modules that we should have, I can close or modify it without any problem.

@fardavide fardavide removed their request for review September 27, 2023 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants