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.

Sign up for GitHub

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

Jump to bottom

Gradle rework and version updates #114

Merged
merged 6 commits into from
Jan 16, 2023
Merged

Gradle rework and version updates #114

merged 6 commits into from
Jan 16, 2023

Conversation

mpetuska
Copy link
Contributor

  • Update to latest kotlin and android plugins.
  • Replace buildSrc with convention plugins
  • Update samples with latest android APIs and remove deprecations
  • Add refreshVersions for dependency management
  • Add missing kotlin targets where appropriate
  • Add android aar artefact publishing

patjackson52
patjackson52 reviewed Jan 14, 2023
View reviewed changes
Copy link
Contributor

@patjackson52 patjackson52 left a comment

Choose a reason for hiding this comment

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

This is great! thanks for putting the effort for updating these deps and migrating to refreshDeps.
Review is a little difficult since its mostly gradle file changes and restructuring. Why is it neccessary for the threadsafe move?
Overall looks good once the builds and tests are passing.

@mpetuska
Copy link
Contributor Author

This is great! thanks for putting the effort for updating these deps and migrating to refreshDeps. Review is a little difficult since its mostly gradle file changes and restructuring. Why is it neccessary for the threadsafe move? Overall looks good once the builds and tests are passing.

Threadsafe package had to move since we now publish proper android release and debug artefacts so each lib should have unique namespace.

@mpetuska
Copy link
Contributor Author

FYI: I have more changes ready for this pr, but I cannot push them due to branch protection rules. Could you look into it, please?

@mpetuska mpetuska force-pushed the gradle-rework-and-version-updates branch 4 times, most recently from 0a17bf0 to 48beaa8 Compare January 16, 2023 11:24
@mpetuska
Copy link
Contributor Author

Made some initial adjustments to CI to support new setup, but the main work for it will come in the followup PR. Current state should only support detekt/ktlint check and snapshot releases from PR.

@mpetuska mpetuska marked this pull request as ready for review January 16, 2023 11:26
@mpetuska mpetuska force-pushed the gradle-rework-and-version-updates branch from 48beaa8 to 269dd90 Compare January 16, 2023 11:36
@mpetuska mpetuska enabled auto-merge (rebase) January 16, 2023 11:51
@mpetuska
Copy link
Contributor Author

@patjackson52 modified CI slightly to work with updated setup so now this PR is ready for your final review. I'll raise a separate PR for the rest of CI changes.

patjackson52
patjackson52 reviewed Jan 16, 2023
View reviewed changes
...-kotlin-threadsafe/src/commonMain/kotlin/org/reduxkotlin/threadsafe/CreateThreadSafeStore.kt
reducer: Reducer<State>,
preloadedState: State,
enhancer: StoreEnhancer<State>? = null
public fun <State> createThreadSafeStore(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is a new lint check or something in newer versions of kotlin? public is implicit

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've enabled explicitApi() in kotlin configs which errors if visibility or public types are not defined. Makes it easier to look at sources from lib consumer side.

patjackson52
patjackson52 reviewed Jan 16, 2023
View reviewed changes
patjackson52
patjackson52 reviewed Jan 16, 2023
View reviewed changes
Copy link
Contributor

@patjackson52 patjackson52 left a comment

Choose a reason for hiding this comment

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

great stuff. left a few comments to discuss before approval.

@mpetuska
Copy link
Contributor Author

Indentation switched to 4 spaces

patjackson52
patjackson52 approved these changes Jan 16, 2023
View reviewed changes
Copy link
Contributor

@patjackson52 patjackson52 left a comment

Choose a reason for hiding this comment

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

Thanks for being flexible and this major step on getting this repo up to date!

@mpetuska mpetuska merged commit cdc1904 into master Jan 16, 2023
@mpetuska mpetuska deleted the gradle-rework-and-version-updates branch January 16, 2023 19:13
@mpetuska mpetuska mentioned this pull request Jan 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patjackson52 patjackson52 patjackson52 approved these changes

Assignees

@patjackson52 patjackson52

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mpetuska @patjackson52