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

Demo app foundations #24

Merged
merged 2 commits into from
Jan 18, 2024
Merged

Demo app foundations #24

merged 2 commits into from
Jan 18, 2024

Conversation

hamorillo
Copy link
Contributor

@hamorillo hamorillo commented Jan 17, 2024

Creating the DemoApp foundations. As we've decided to use Jetcpack Compose adding also the basic dependencies.

Note: Composable functions start with uppercase, so Ktlint detects them as an error. I'm adding the exception for this case in the .editorconfig.

Without the exception, you'll receive the following lint error:

Function name should start with a lowercase letter (except factory methods) and use camel case (cannot be auto-corrected)

I saw that Twitter has a repo with more rules; just commenting on it here for future reference.

Testing

  • - Verify you can compile and launch the DemoApp (You should see a really nice Hello Android! 😅)

The DemoApp will be created with JetpackCompose so that the basic dependencies will be added.
Composable functions start with uppercase, so Ktlint detects it as an error. Adding the exception for this case.

"Function name should start with a lowercase letter (except factory methods) and use camel case (cannot be auto-corrected)"

More configurations can be added using the .editoconfig file. See: https://pinterest.github.io/ktlint/latest/rules/configuration-ktlint/ and https://editorconfig.org/
@hamorillo hamorillo requested a review from maxme January 17, 2024 16:48
@maxme
Copy link
Contributor

maxme commented Jan 18, 2024

I saw that Twitter has a repo with more rules; just commenting on it here for future reference.

The official ktlint doc also refers to this Compose ruleset

Copy link
Contributor

@maxme maxme left a comment

Choose a reason for hiding this comment

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

LGTM, feel free to ignore the comment and :shipit:

) {
val colorScheme =
when {
dynamicColor && Build.VERSION.SDK_INT >= Build.VERSION_CODES.S -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

modifier: Modifier = Modifier,
) {
Text(
text = "Hello $name!",
Copy link
Contributor

@maxme maxme Jan 18, 2024

Choose a reason for hiding this comment

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

Maybe I'm nitpicking because we'll likely remove this soon, but should we start with localized strings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will delete it in the following PR, so it won't last long. But yeah, the following strings should be localized.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hamorillo BTW, do we have a rule to check for unlocalizable strings?

@hamorillo
Copy link
Contributor Author

The official ktlint doc also refers to this Compose ruleset

In fact, I've found the Twitter repo because Compose ruleset is a fork of it. 😅

@hamorillo
Copy link
Contributor Author

❤️ Thanks for the review @maxme!

@hamorillo hamorillo merged commit 5c5e33a into trunk Jan 18, 2024
1 check passed
@hamorillo hamorillo deleted the hamorillo/1-demo-app-foundations branch January 18, 2024 09:18
@hamorillo hamorillo mentioned this pull request Jan 18, 2024
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