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

2065 local forging support new operations for lima #2106

Closed

Conversation

hui-an-yang
Copy link
Collaborator

Thank you for your contribution to Taquito.

Before submitting this PR, please make sure:

  • Your code builds cleanly without any errors or warnings
  • You have added unit tests
  • You have added integration tests (if relevant/appropriate)
  • All public methods or types have TypeDoc coverage with a complete description, and ideally an @example
  • You have added or updated corresponding documentation
  • If relevant, you have written a first draft summary describing the change for inclusion in Release Notes.

Release Note Draft Snippet

If relevant, please write a summary of your change that will be suitable for
inclusion in the Release Notes for the next Taquito release.

@github-actions
Copy link

github-actions bot commented Nov 8, 2022

New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/.

Published packages:

npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/

@codecov-commenter
Copy link

codecov-commenter commented Nov 8, 2022

Codecov Report

❗ No coverage uploaded for pull request base (lima@ecade81). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             lima    #2106   +/-   ##
=======================================
  Coverage        ?   81.34%           
=======================================
  Files           ?      222           
  Lines           ?    12412           
  Branches        ?     2781           
=======================================
  Hits            ?    10097           
  Misses          ?     2293           
  Partials        ?       22           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

hui-an-yang and others added 11 commits November 8, 2022 11:03
* Fix taquito-test-dapp build when using local packages

* Add build script for test-dapp to root package.json

* Commit apps/taquito-test-dapp/package-lock.json

* Exclude @taquito/website and taquito-test-dapp-vite from nx build command

* Fix deploy_website pipeline

* Re-link local dependencies when deploying on Netlify

* Only run Taquito packages unit tests

* Remove empty NPM test script from pack-test-tool

* Rebuild dependencies when on Netlify

* Use workspaces

* Remove prebuild-test-dapp script

* Fix taquito-test-dapp build

* Install lerna in apps/taquito-test-dapp

* Trigger Netlify cache deploy

* Remove typeRoots propriety from every tsconfig.json

* Fix dependencies for website

* Delete unused package-lock.json

* Correctly link local dependencies

* Fix taquito-http-utils

* Replace docusaurus2-dotenv with docusaurus-plugin-dotenv

* Use NPM workspaces to run commands

* Fix Jest testResultsProcessor path

* Revert webpack upgrade in @taquito/taquito

* Update package-lock.json

* Update package-lock.json

* Upgrade @taquito/taquito to webpack5

* Update codecov Github Action

* Trigger build

* Remove deprecated Jakartanet testnet from Taquito Test Dapp

* Update root Readme.md file with the latest changes for building Taquito

Co-authored-by: roxaneletourneau <[email protected]>
Bumps [loader-utils](https://github.com/webpack/loader-utils) from 1.4.1 to 1.4.2.
- [Release notes](https://github.com/webpack/loader-utils/releases)
- [Changelog](https://github.com/webpack/loader-utils/blob/v1.4.2/CHANGELOG.md)
- [Commits](webpack/loader-utils@v1.4.1...v1.4.2)

---
updated-dependencies:
- dependency-name: loader-utils
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
@hui-an-yang hui-an-yang marked this pull request as ready for review November 18, 2022 21:48
@hui-an-yang hui-an-yang linked an issue Nov 18, 2022 that may be closed by this pull request
Copy link
Collaborator

@roxaneletourneau roxaneletourneau left a comment

Choose a reason for hiding this comment

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

Good work, some minor comments about the tests.

Can you open an issue to remind us to delete "taquito-local-forging/src/proto14-kathmandu/" when we will be working on M protocol?


describe(`Test origination of a token contract using: ${rpc}`, () => {

beforeEach(async (done) => {
await setup();
done()
})
test('Originates a contract having ticket with init and the wallet api', async (done) => {
kathmandunet('Originates a contract having ticket with init and the wallet api', async (done) => {
Copy link
Collaborator

@roxaneletourneau roxaneletourneau Nov 18, 2022

Choose a reason for hiding this comment

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

I think this specific test is duplicated from "integration-tests/wallet-edo-deploy-having-ticket.spec.ts". Can it be removed?

await setup();
done()
})
limanetAndAlpha('Originates a contract having ticket with init and the wallet api', async (done) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same for this one that seems to be duplicated in integration-tests/wallet-edo-deploy-having-ticket.spec.ts


export const limaCases: TestCase[] = [
// In `opMapping` from the file `constants.ts`, the operations and types starting at `ticket` were added in the lima protocol
...extractOp(154, 154, opMapping).map((op): TestCase => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we test from 152 to 154 to include lambda_rec as well?

@@ -1345,7 +1347,7 @@ export const commonCases: TestCase[] = [
];

export const kathmanduCases: TestCase[] = [
...extractOp(150, 151, opMapping).map((op): TestCase => {
...extractOp(150, 151, opMappingProto14).map((op): TestCase => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move the following tests:

  • Origination of a contract that contains the instructions EMIT
  • Increase Paid Storage operation
    into commonCases ? Those operation/instruction are now supported on every protocol we are testing against (Kathmandu, Lima, alpha and upcoming)

@@ -17,7 +17,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16.13.1
node-version: 16
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the reasoning behind this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

is this perhaps from master?

@dsawali
Copy link
Contributor

dsawali commented Nov 21, 2022

Does this branch have changes from the recent master?

@hui-an-yang hui-an-yang linked an issue Nov 22, 2022 that may be closed by this pull request
@hui-an-yang hui-an-yang deleted the 2065-local-forging-support-new-operations-for-lima branch November 23, 2022 21:53
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.

Local-forging: Support new encoding for TICKET for Lima
5 participants