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

Introduce abstraction for CurrencyId into farming pallet #960

Conversation

RustNinja
Copy link
Contributor

Farming Pallet is very useful and this PR focused to Introduce abstraction for currency id into farming pallet via loose pallet coupling that is more flexible and preferable in most cases.

Here is the changes that was added:

  • introduce associated type CurrencyId for farming pallet
  • update all runtimes (Polkadot/Kusama)
  • update unit tests and mock runtimes

Once this changes was merged anyone will able to use farming pallet in their runtimes.

@ark930
Copy link
Member

ark930 commented Apr 25, 2023

Run command SKIP_WASM_BUILD= cargo check -p node-cli --locked --features "with-all-runtime" failed

@gianfra-t
Copy link
Contributor

gianfra-t commented Nov 7, 2023

Hi @ark930, we have fixed the failing command and merged with the latest dev branch. Running tests locally seems to work okay, could you please give it a try at the CI?

@gianfra-t
Copy link
Contributor

Sorry it failed with check. Must have broken with the merge.
Could you please tell me what commands does the CI run so I can test all of them locally?

@ark930
Copy link
Member

ark930 commented Dec 25, 2023

@gianfra-t make check-all && make clippy && make test-all

@gianfra-t
Copy link
Contributor

Thanks @ark930, I have replaced the currency id in the benchmarks for the default so it is compatible with the generic implementation. I believe this should not introduce any fundamental difference, but please let me know.

@ark930 ark930 merged commit 46ba368 into bifrost-io:develop Dec 29, 2023
1 check failed
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