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/add local build instructions to readme #58

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

Ryanmtate
Copy link
Contributor

No description provided.

@Ryanmtate Ryanmtate changed the base branch from feat/oid4vp-jwt-vc-support to skit-459-integrate-oid4vp-into-mobile-sdk-rust-swift-kotlin October 18, 2024 03:11
@Ryanmtate Ryanmtate force-pushed the feat/add-local-build-instructions-to-readme branch from a9343d2 to bc98e43 Compare October 18, 2024 15:05
Comment on lines 122 to 130
debug(MavenPublication) {
groupId = 'com.spruceid.mobile.sdk.rs'
artifactId = "mobilesdkrs"
version = System.getenv("VERSION")

afterEvaluate {
from components.release
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: I am adding this back because it is needed for releasing to mavenLocal() repository.

Copy link
Contributor

@theosirian theosirian Oct 21, 2024

Choose a reason for hiding this comment

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

Just for reference, for this to be able to stay in the repo, the release action in the CI needs to be updated use individual publish calls, that is, ./gradlew publishReleasePublicationToGitHubPackagesRepository and ./gradlew publishReleasePublicationToNmcpReleaseRepository instead of ./gradlew publish.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, this is good to know! 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does publishReleasePublicationToCentralPortal provide support for Nmcp?

I didn't add publishReleasePublicationToNmcpReleaseRepository step to the release, but I did replace publish with publishReleasePublicationToGitHubPackagesRepository in 77e35df

@Ryanmtate Ryanmtate marked this pull request as ready for review October 18, 2024 15:21
@Ryanmtate Ryanmtate mentioned this pull request Oct 18, 2024
@Ryanmtate Ryanmtate force-pushed the feat/add-local-build-instructions-to-readme branch from ebf1d4a to aca62b4 Compare October 21, 2024 04:48
Copy link
Contributor

@rschulman rschulman left a comment

Choose a reason for hiding this comment

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

One comment but generally LGTM.

cd kotlin/ && VERSION=x.y.z ./gradlew publishDebugPublicationToMavenLocal
```

Where `VERSION` is set to a SemVer (Semantic Versioning). Note that it is possible to use a tagged version, e.g. `0.0.33-SNAPSHOT`, which may be preferrable to denote
Copy link
Contributor

Choose a reason for hiding this comment

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

I might make this language even stronger. Something like "It is strongly advised to use a tagged version, as using a regular version can cause confusion down the line when the local maven repository contains conflicting version numbers." (Yes, this has bit me in the past).

@Ryanmtate Ryanmtate force-pushed the skit-459-integrate-oid4vp-into-mobile-sdk-rust-swift-kotlin branch from f17fe1f to dfb7b39 Compare October 22, 2024 14:39
Base automatically changed from skit-459-integrate-oid4vp-into-mobile-sdk-rust-swift-kotlin to main October 22, 2024 15:45
@Ryanmtate Ryanmtate force-pushed the feat/add-local-build-instructions-to-readme branch from 77e35df to 0005376 Compare October 22, 2024 16:42
@Ryanmtate Ryanmtate merged commit bd8c47f into main Oct 22, 2024
3 checks passed
@Ryanmtate Ryanmtate deleted the feat/add-local-build-instructions-to-readme branch October 22, 2024 18:21
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.

3 participants