TBD54566975 / web5-kt

Apache License 2.0
7 stars 9 forks source link

Convert Gradle to Maven Build #261

Closed ALRubinger closed 3 months ago

ALRubinger commented 4 months ago

Summary

This PR creates a mechanism whereby we may align dependencies between web5-kt and projects built atop it, (ie. tbdex-kt). This satisfies #217. It does so by replacing the existing Gradle build with a Maven one and adding explicit <dependencyManagement> sections through which dependency versioning can be governed and exported for use in other projects. A more in-depth account of that reasoning is here on Discord.

This is offered as a drop-in replacement for the existing build:

Expected impact for devs of web5-kt is in 3 dimensions:

API Docs

Kover and CodeCov

Detekt

[INFO] --- detekt:1.23.5:check (default) @ web5-common ---
[INFO] Args:
.........

9 kotlin files were analyzed.
Complexity Report:
    - 941 lines of code (loc)
    - 590 source lines of code (sloc)
    - 425 logical lines of code (lloc)
    - 256 comment lines of code (cloc)
    - 122 cyclomatic complexity (mcc)
    - 71 cognitive complexity
    - 0 number of total code smells
    - 43% comment source ratio
    - 287 mcc per 1,000 lloc
    - 0 code smells per 1,000 lloc

Project Statistics:
    - number of properties: 97
    - number of functions: 36
    - number of classes: 12
    - number of packages: 1
    - number of kt files: 9

Release

Example Releases

Release on TBD Artifactory

This is triggered by the "Release and Publish" workflow. This example has version: 0.15.0-issue217-alpha-1

GitHub Release

Same as the example above: a GitHub Release for: 0.15.0-issue217-alpha-1

SNAPSHOT on TBD Artifactory

These are published on every push to main. This example has version: commit-2ed97da-SNAPSHOT

Release Candidate Build on TBD Artifactory

Published: https://blockxyz.jfrog.io/artifactory/tbd-oss-releases-maven2/xyz/block/web5/1.0.0-maven-rc-1/

Release on Maven Central

ALRubinger commented 4 months ago

Update on how this is going @jiyoontbd @frankhinek:

Still TODO:

But this is an encouraging stopping point for today. Thanks for resolving that cyclic dependency. Found no other issues in Mavenizing. Looks like I won't need much from @jiyoontbd until we do review of this chonky PR. 🤘🏻

ALRubinger commented 4 months ago

Things that will require some validation and perhaps follow-on commits after this is merged:

These we can't fully test until they're in main, but in case of errors we can fix them up after. I've done my best to validate the commands locally first.

ALRubinger commented 4 months ago

Added Dokka config using Dokka CLI as Dokka Maven plugin does not support multimodule builds. The config looks a little different from before, with no sidebar. Gotta be "good enough" for now, though I'm very open to anyone who can restore the left nav sidebar.

image

vs. before:

image
leordev commented 4 months ago

Added Dokka config using Dokka CLI as Dokka Maven plugin does not support multimodule builds. The config looks a little different from before, with no sidebar. Gotta be "good enough" for now, though I'm very open to anyone who can restore the left nav sidebar.

Does the sidebar shows up when one of the modules is selected? If so, I think we should be ok for now, since they are different packages. Are there any interlinked docs? For example, in the dids package, a link to a crypto class/definition that does not work anymore?

ALRubinger commented 4 months ago

Added Dokka config using Dokka CLI as Dokka Maven plugin does not support multimodule builds. The config looks a little different from before, with no sidebar. Gotta be "good enough" for now, though I'm very open to anyone who can restore the left nav sidebar.

Does the sidebar shows up when one of the modules is selected? If so, I think we should be ok for now, since they are different packages. Are there any interlinked docs? For example, in the dids package, a link to a crypto class/definition that does not work anymore?

@leordev It doesn't, boo. I revised the README to include "Generating API Docs Locally" if you wanna give it a spin. Note the Hermit installation and source ./bin/activate-hermit steps which are new in main and I've rebased into here.

Would you mind checking those instructions work for you, and see if the interlinking works as you're expecting?

ALRubinger commented 4 months ago

Update: Getting close. Need to reconcile how we publish to Maven Central now. All publishing in this PR is to TBD Artifactory as it stands. Will link with @leordev on that tomorrow.

Also TODO: The parent POM here doesn't actually export all the web5 submodules as deps. Fix this structurally so that consumers have one dep declaration to make and it brings in everything.

I squashed everything and rebased atop main to keep things clean and make further rebasing easier.

ALRubinger commented 4 months ago

I think we have to run Kover to generate the reports that CodeCov reads, good catch from @leordev

ALRubinger commented 4 months ago

Doesn't look like the cache on the build-test-deploy script is doing anything; lots of Maven Central downloads there. Why?

ALRubinger commented 4 months ago

Things that will require some validation and perhaps follow-on commits after this is merged:

  • Publish Action
  • Publish SNAPSHOT Action

These we can't fully test until they're in main, but in case of errors we can fix them up after. I've done my best to validate the commands locally first.

This is no longer true. I've updated the workflows here to run on this branch and they're tested on this PR thanks to @leordev showing me how to do that.

codecov[bot] commented 4 months ago

Codecov Report

Merging #261 (c717caf) into main (f3e8a88) will decrease coverage by 0.04%. Report is 4 commits behind head on main. The diff coverage is n/a.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #261 +/- ## ========================================== - Coverage 75.78% 75.75% -0.04% ========================================== Files 43 42 -1 Lines 1900 1856 -44 Branches 342 342 ========================================== - Hits 1440 1406 -34 + Misses 313 303 -10 Partials 147 147 ``` | [Components](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | Coverage Δ | | |---|---|---| | [credentials](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `80.70% <100.00%> (-0.30%)` | :arrow_down: | | [crypto](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `43.93% <ø> (-0.19%)` | :arrow_down: | | [dids](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `86.28% <100.00%> (-0.26%)` | :arrow_down: | | [common](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `66.29% <ø> (ø)` | |
ALRubinger commented 4 months ago

Boom Kover works now.

https://app.codecov.io/github/TBD54566975/web5-kt/tree/issue-217%2Fmaven-build

image
ALRubinger commented 4 months ago

For testing of GitHub Actions, I'm continuing work on this branch in my fork on main; will bring back here after I can validate Actions are working correctly.

https://github.com/ALRubinger/web5-kt/

codecov-commenter commented 3 months ago

Codecov Report

Merging #261 (dec79ab) into main (f3e8a88) will decrease coverage by 2.25%. Report is 17 commits behind head on main. The diff coverage is n/a.

:exclamation: Current head dec79ab differs from pull request most recent head 15bcba7. Consider uploading reports for the commit 15bcba7 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #261 +/- ## ========================================== - Coverage 75.78% 73.54% -2.25% ========================================== Files 43 45 +2 Lines 1900 2158 +258 Branches 342 401 +59 ========================================== + Hits 1440 1587 +147 - Misses 313 376 +63 - Partials 147 195 +48 ``` | [Components](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | Coverage Δ | | |---|---|---| | [credentials](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `81.12% <82.60%> (+0.12%)` | :arrow_up: | | [crypto](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `49.16% <58.20%> (+5.04%)` | :arrow_up: | | [dids](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `80.50% <70.84%> (-6.04%)` | :arrow_down: | | [common](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/261/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `65.21% <25.00%> (-1.09%)` | :arrow_down: |
ALRubinger commented 3 months ago

Add to README - Snapshots are published using version format: commit-{shortSHA}-SNAPSHOT. @leordev will update.