-
Notifications
You must be signed in to change notification settings - Fork 37
move signify repository into keria #398
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
base: main
Are you sure you want to change the base?
Conversation
Add in attachment serialization functions
…ex-script add script for single issuer and single holder ipex
Update challenge.ts
remove unused dependencies, move dev dependencies
…oint Remove name param from Credentials.list()
Multisig grant and admit messages and fixes in seqner
Signed-off-by: arshdeep singh <arsh.binny@gmail.com>
use esbuild, tsc and eslint instead of tsdx
…entials-ipex Update an integration example for credentials to support ipex
add join function in groups class to join multisig rotation event
running integration tests
add runInBand to avoid concurrency issues add salty, external module tests
* spike: Feasibility of generating Signify types from Python type hints via OpenAI * add and use generated TS types * demo generated types for credentials().get() * update types for CredentialState * resolve review comments * fix credentials types + add registries's types * Add more types for credentialing * resolve review comments * fix errors/warning for swagger validator * re-generate types * re-generate types * remove debug logs and add multiple overload signatures * separate generate:types and build * fix formatting * use the node.js API instead of starting a child process for generating openAPI types * remove unuse schemas * resolve review comment: fix the eslint rules * update package-lock.json * update types since keria changed * update new types from keria * aiding.py type hints and auto-generated OpenAPI specs * Generate types for Group Member * resolve review comments * fix mock data for aiding test * resolve review comments * resolve review comments * convert old mockdata + recreate KEL types (all fields are required) * rebase and update code * remove unuse test file * update keria api schema * fix audit * resolve review comments: improve ts types for tests of aiding
6ca00cc to
90d30ec
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #398 +/- ##
==========================================
- Coverage 86.60% 86.54% -0.07%
==========================================
Files 25 25
Lines 5286 5298 +12
==========================================
+ Hits 4578 4585 +7
- Misses 708 713 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
90d30ec to
1f053bd
Compare
|
I have never done this but it might be worth experimenting to see if there's a way to merge the git histories. From a quick search, there seems to be a few options. Any thoughts? |
Agreed. Either we can do it after, or fix Signify on the main branch now. With respect to other changes: As both a sanity and security check, I'd like to take a hash of the signify-ts repo directory locally and compare against the new directory being introduced here. So I would avoid updating anything inside the folder (e.g. docs) until another PR. cc @m00sey @kentbull |
I have removed package-lock.json and used in the root instead. Also fixed the linting config. But I can revert both of those changes. Otherwise a hash of the src directory would maybe suffice. |
Sure, that works for me! |
1f053bd to
92be709
Compare
|
@iFergal I have now included the commit history from signify-ts. |
19d1d66 to
d35f8e2
Compare
…eType' object has no attribute 'ked' (WebOfTrust#379)" This reverts commit 47e424b.
fd4641e to
64eda44
Compare
64eda44 to
a51123e
Compare
iFergal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done a diff against signify-ts main and all LGTM!
|
|
||
| - name: Extract version | ||
| run: | | ||
| echo "VERSION=$(./version.sh)" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case we deploy a release a new version of KERIA that has no changes in Signify - will npm artifacts get overiden with newer builds? And vice versa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NPM does not allow overwriting, you'd have to change the version. In the particular case you mentioned both signify and keria would get a new release (I.e. x.y.z-dev.commitsha). In case of 'latest' releases, you'd have to increment the version in pyproject.toml between each publish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK - docker registries probably do though right? (in case Signify is bumped but not KERIA)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, Docker hub and GHCR does anyway. But, just to be clear, the workflow I wrote for publishing will use the same version for signify-ts and keria.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I looked a bit too quickly and didn't realise it was applied to both.
Won't that become problematic if we want semantic versioning on APIs? I don't expect every PR will have KERIA and Signify changes - there may be bug fixes or breaking changes in either that don't impact the other (or aren't breaking, for example)
|
Do we want to do one last release of KERIA and Signify with all latest changes before we do this merge? The delegation fixes I have in a draft PR to KERIpy 1.2.7, that also need to go in to main and 1.3.2, need to be merged to KERIpy, then a new KERIA version with the new KERIpy version. What do you think about us doing all of that and then merging this? I like the idea of processing all in-flight work, except ESSR and maybe some old PRs, and then doing this repo merge. |
Fine by me. I will wait doing any rebasing until then so I don't duplicate work. It's not very complicated though. Biggest problem was failing integration tests in signify-ts. If we can ensure that the tests pass against the release, this PR will be even easier to re-create as no source or test code will have to change. |
Per discussion on discord.
This PR moves the signify-ts into keria repository. I have included the commit history from signify-ts repository my merging with "--allow-unrelates-histories". The purpose is to use signify-ts as a reference client implementation, they are to be released and tested together as a unit.
mainon signify-tsPotential todos, can also be subsequent PRs
The failing integration tests are probably also failing on signify-ts if they would run against latest keria. So I am not sure if that needs to be part of this PR. I have fixed the tests on another branch, but it contains a lot more changes that we probably do not want to add without reviewing.
When I set up the docker-compose, I opted for using network_mode: host. This enable you to switch between running keria or the witness-demo as containers, or locally, without updating any configuration or setting environment variables. On docker desktop, you might have to enable this.