Feature/rome constatinople migration test #319
Conversation
mnaamani
left a comment
mnaamani
left a comment
There was a problem hiding this comment.
I was able to run the test-migration successfully. I left a few comments about re-organizing some of the folder structure, but I'm okay with these changes being made later as there are several other PRs depending on this.
The main issue that needs to be resolved here however is how to properly have dependencies mutltiple different versions of our "types" package.
| "dependencies": { | ||
| "@joystream/types": "^0.7.0", | ||
| "@joystream/types": "../joystream-apps/packages/joy-types", | ||
| "@rome/types@npm:@joystream/types": "^0.7.0", |
There was a problem hiding this comment.
I see the first "@joystream/types": "../joystream-apps/packages/joy-types", is just a relative path to the latest constantinople @joystream/types package.
But I wanted to confirm my understanding, "@rome/types@npm:@joystream/types" is setting some sort of alias from @rome/types to @joystream/types published on npm ?
There was a problem hiding this comment.
Would it be possible to do something like:
"@constantinople/types@npm:@joystream/types": "^0.8.0",
"@rome/types@npm:@joystream/types": "^0.7.0",
This might be useful but what would be more helpful is to see if we can have two packages in the same workspace setup the same alias to two different versions of the same package name?
There was a problem hiding this comment.
We cannot merge with package.json having such a relative path to a dependency.
How can we tackle this.
There was a problem hiding this comment.
I see the first "@joystream/types": "../joystream-apps/packages/joy-types", is just a relative path to the latest constantinople @joystream/types package.
But I wanted to confirm my understanding, "@rome/types@npm:@joystream/types" is setting some sort of alias from
@rome/typesto@joystream/typespublished on npm ?
Exactly, it's an alias.
Would it be possible to do something like:
"@constantinople/types@npm:@joystream/types": "^0.8.0", "@rome/types@npm:@joystream/types": "^0.7.0",
Yes, after .8 will be published to nexus
We cannot merge with package.json having such a relative path to a dependency.
How can we tackle this.
We can either publish .8 types, or I can put a placeholder here for the relative path.
| import { membershipTest } from './membershipCreationTest'; | ||
| import { KeyringPair } from '@polkadot/keyring/types'; | ||
| import { ApiWrapper } from '../utils/apiWrapper'; | ||
| import { ApiWrapper } from '../../utils/apiWrapper'; |
There was a problem hiding this comment.
It probably makes sense to move the top utils folder inside constantinople folder (like rome) so it will be clear which code is for which version of the network.
| // Proposal creation | ||
| const proposalPromise = apiWrapper.expectProposalCreated(); | ||
| await apiWrapper.proposeRuntime( | ||
| await apiWrapper.proposeRuntimeRome( |
There was a problem hiding this comment.
I'm not so sure about adding Rome suffix here in the names of the functions. It doesn't seem necessary if we are separating the code for different networks into separate folders.
| } | ||
|
|
||
| public static readRuntimeFromFile(path: string): string { | ||
| return '0x' + fs.readFileSync(path).toString('hex'); |
There was a problem hiding this comment.
Why is this function necessary. In other words, why are we having to pass the wasm blob as a hex string in the call to create the runtime upgrade proposal. Isn't it possible to pass the Buffer type or at least convert it more efficiently to the required Codec type.
There was a problem hiding this comment.
I will give it a try and will push the changes.
There was a problem hiding this comment.
I found no way to pass the Buffer, and changed the conversion to
u8aToHex(...)
But the object passed to API is still a hex string.
2 participants
No description provided.