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: added improved gh actions #293

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,57 @@ jobs:
library-x86_64-linux-android
failOnError: false

alpha-release:
name: Alpha Release
if: github.event_name == 'push'
wadeking98 marked this conversation as resolved.
Show resolved Hide resolved
permissions:
id-token: write
runs-on: ubuntu-latest
env:
NPM_TAG: 'alpha'
needs:
- create-ios-xcframework
- create-android-library
Comment on lines +438 to +440
Copy link
Member

Choose a reason for hiding this comment

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

This seems to just release the ios/android wrappers as alpha. Not the Node.JS.

I would really like to keep consistency between node.js and react native.

My proposal for this PR would be:

  • on push to main, build the binaries for indy vdr
  • update the version of the wrapper to the current package.json version but with e.g. .alpha-{commit-sha} appended
  • create a prerelease on github and push the binaries
  • release the JS wrapper

Copy link
Member

Choose a reason for hiding this comment

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

The github pre releaase could e.g. be called "JS 0.2.2 Alpha {sha-commit}" to make it clear this is a JS pre-release.

Then the package.json binary is updated to link to this pre-release.

This way pre-releases work the same as normal releases.

Copy link
Contributor

@cvarjao cvarjao Jun 6, 2024

Choose a reason for hiding this comment

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

It seems messy to manage different lifecycle in the same repository. I think we need to move to uniffi-rs or some other tool. I am concern about those wrappers getting out of sync, missing releases, and release from side branches, like we it was done.

defaults:
run:
working-directory: ./wrappers/javascript
timeout-minutes: 7
steps:
Copy link

Choose a reason for hiding this comment

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

A few spaces won't change much but make make it easier for the eye to follow. Between steps?

- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/setup-node@v4
with:
node-version: '20.x'
registry-url: 'https://registry.npmjs.org'
cache: "yarn"
cache-dependency-path: "wrappers/javascript"
- name: install
run: yarn install
wadeking98 marked this conversation as resolved.
Show resolved Hide resolved

- name: Fetch Android libraries
uses: actions/download-artifact@v4
with:
name: android-libraries
path: indy-vdr-react-native/native/mobile/android/

- name: Fetch iOS Framework
uses: actions/download-artifact@v4
with:
name: indy_vdr.xcframework
path: indy-vdr-react-native/native/mobile/ios/

# On push to main, release unstable version
- name: Release alpha
run: |
git update-index --assume-unchanged $(find . -type d -name node_modules -prune -o -name 'package.json' -print | tr "\n" " ")
#export NEXT_VERSION_BUMP=$(yarn next-version-bump)
export NEXT_VERSION_BUMP="major"
Copy link
Member

Choose a reason for hiding this comment

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

This will always increase the version by major, not sure if that's what we want?

Maybe we can just do the current version + alpha-{commit-sha}?

npx lerna publish --no-verify-access --no-private --loglevel=verbose --canary $NEXT_VERSION_BUMP --dist-tag $NPM_TAG --force-publish --yes
env:
NPM_TOKEN: ${{ secrets.NPM_PUBLISH }}
NODE_AUTH_TOKEN: ${{ secrets.NPM_PUBLISH }}

create-ios-android-release-asset:
name: Create iOS and Android release assets
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion wrappers/javascript/indy-vdr-nodejs/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@hyperledger/indy-vdr-nodejs",
"version": "0.2.0-dev.6",
"version": "0.2.2",
"license": "Apache-2.0",
"description": "Nodejs wrapper for Indy Vdr",
"source": "src/index",
Expand Down

This file was deleted.

16 changes: 4 additions & 12 deletions wrappers/javascript/indy-vdr-react-native/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@hyperledger/indy-vdr-react-native",
"version": "0.2.0-dev.6",
"version": "0.2.2",
"license": "Apache-2.0",
"description": "React Native wrapper for Indy Vdr",
"source": "src/index",
Expand Down Expand Up @@ -28,6 +28,7 @@
"ios/indyVdr.xcodeproj/project.pbxproj",
"cpp/**/*.cpp",
"cpp/**/*.h",
"native/**",
Copy link
Member

Choose a reason for hiding this comment

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

This should not be published

"indy-vdr.podspec",
"README.md",
"LICENSE",
Expand All @@ -36,12 +37,10 @@
"scripts": {
"build": "yarn clean && yarn compile",
"clean": "rimraf -rf ./build",
"compile": "tsc -p tsconfig.build.json",
"install": "node-pre-gyp install"
"compile": "tsc -p tsconfig.build.json"
},
"dependencies": {
"@hyperledger/indy-vdr-shared": "0.2.0-dev.6",
"@mapbox/node-pre-gyp": "^1.0.10"
"@hyperledger/indy-vdr-shared": "0.2.0-dev.6"
Copy link
Member

Choose a reason for hiding this comment

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

I believe this needs updating to 0.2.2?

},
"devDependencies": {
"@types/react": "^16.9.19",
Expand All @@ -54,12 +53,5 @@
"peerDependencies": {
"react": ">= 16",
"react-native": ">= 0.66.0"
},
"binary": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this break the non-alpha releases? My preference would really be to fetch the libraries from gh in a post install (what node-pre-gyp does), but if there are benefits I am okay with it.

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 got rid of this because the new github actions copy the compiled binaries into the native folder, so the alpha release will target the latest binaries

Copy link
Contributor

Choose a reason for hiding this comment

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

we can go back to fetch when/if we can figure out publishing alpha releases of the rust libraries. I was thinking on maybe publishing alphas builds to GitHub Package Registry (maybe as maven artifacts)

Copy link
Contributor

@berendsliedrecht berendsliedrecht Jun 5, 2024

Choose a reason for hiding this comment

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

I got rid of this because the new github actions copy the compiled binaries into the native folder, so the alpha release will target the latest binaries

but it only does this in the alpha-release action, correct? Normal releases will not contain the copied library and fail as we also do not push them to the repository.

we can go back to fetch when/if we can figure out publishing alpha releases of the rust libraries. I was thinking on maybe publishing alphas builds to GitHub Package Registry (maybe as maven artifacts)

I would stay away from maven artifacts, or pods, as it will change the setup process quite a bit. Might be a nice end goal, but for now just including the xcframework and android libraries like so is fine.

For rust alpha releases, something like 0.1.0-alpha.1 will work on crates.io.

Copy link
Member

Choose a reason for hiding this comment

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

Honestly i don't see a reason to change this setup?

It works cross platform for Node.JS / iOS / Android and means you only download the binary for your platform (in the Node.JS release)

We've put quite some thought into this process and I would be really disappointed to see it replaced by something else that caters better to your use case (react native), but doesn't take into account Node.Js

Copy link
Member

Choose a reason for hiding this comment

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

See my suggestion on how to approach this in the other comment

"module_name": "indy_vdr",
"module_path": "native",
"remote_path": "v0.4.0",
"host": "https://github.com/hyperledger/indy-vdr/releases/download/",
"package_name": "library-ios-android.tar.gz"
}
}
2 changes: 1 addition & 1 deletion wrappers/javascript/indy-vdr-shared/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@hyperledger/indy-vdr-shared",
"version": "0.2.0-dev.6",
"version": "0.2.2",
"license": "Apache-2.0",
"description": "Shared library for using Indy VDR with NodeJS and React Native",
"main": "build/index",
Expand Down
5 changes: 5 additions & 0 deletions wrappers/javascript/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,11 @@
resolved "https://registry.yarnpkg.com/@hutson/parse-repository-url/-/parse-repository-url-3.0.2.tgz#98c23c950a3d9b6c8f0daed06da6c3af06981340"
integrity sha512-H9XAx3hc0BQHY6l+IFSWHDySypcXsvsuLhgYLUGywmJ5pswRVQJUHpOsobnLYp2ZUaUlKiKDrgWWhosOwAEM8Q==

"@hyperledger/[email protected]":
version "0.2.0-dev.6"
resolved "https://registry.yarnpkg.com/@hyperledger/indy-vdr-shared/-/indy-vdr-shared-0.2.0-dev.6.tgz#4954ee06fa8a2e4545b35cd525b7b86e0f10b6fe"
integrity sha512-pNLq0zkqv5rFCpU9tzyJ5DPvED5YE+UFP8iKwVD7fe+mAD6/VpweOunYNKgIBT4K1DYI21q7bs3SzxQZ0hLlKw==

"@isaacs/cliui@^8.0.2":
version "8.0.2"
resolved "https://registry.yarnpkg.com/@isaacs/cliui/-/cliui-8.0.2.tgz#b37667b7bc181c168782259bab42474fbf52b550"
Expand Down
Loading