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

Bump @vocdoni/sdk package from v0.0.16 to v0.4.0 #69

Closed
wants to merge 17 commits into from

Conversation

andreslucena
Copy link
Collaborator

@andreslucena andreslucena commented Sep 26, 2023

I've also added a couple commands that are useful to regenerate and view the database, and an output for the SDK version in the JS console

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #69 (9b37dff) into main (8ea2d6c) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

❗ Current head 9b37dff differs from pull request most recent head ced7380. Consider uploading reports for the commit ced7380 to get more accurate results

@@            Coverage Diff             @@
##             main      #69      +/-   ##
==========================================
- Coverage   92.26%   92.14%   -0.12%     
==========================================
  Files          87       87              
  Lines        1706     1706              
==========================================
- Hits         1574     1572       -2     
- Misses        132      134       +2     
Files Coverage Δ
lib/decidim/vocdoni.rb 100.00% <100.00%> (+3.84%) ⬆️

... and 2 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@andreslucena andreslucena changed the title Bump @vocdoni/sdk package from v0.0.16 to v0.3.1 Bump @vocdoni/sdk package from v0.0.16 to v0.4.0 Oct 11, 2023
@@ -20,7 +20,6 @@ group :development, :test do
end

group :development do
gem "i18n-tasks", "~> 0.9.37"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@microstudi just to be sure, you're removing this as its not necessary, because its already a dependency of decidim-dev, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

@andreslucena
Copy link
Collaborator Author

@microstudi about the NPM packages problems that we had, its already solved upstream, in @vocdoni/sdk. Starting from v0.3.2 assert is already bundled there. I've bumped it to v0.4.0 as that's the last stable.

I've tried it out locally and it seems to work well. The thing is that there's a failing spec, but I think its in vocdoni side, as I see some server/SDK errors in the CI output:

2023-10-11 06:24:35 +0000 SEVERE: http://295.lvh.me:5328/packs-test/js/4538-2dd7a417cce069698b8a.js 1:4401 Uncaught L: 500 : vochain SendTx failed: createAccountTx: payl…b741 != 40867eec6f7929b5148f3418983f9a4096605d31)
2023-10-11 06:25:32 +0000 SEVERE: http://296.lvh.me:5328/packs-test/js/decidim_vocdoni_admin-510e7ba658f699b1a09d.js 1:22098 "RESULT => ERROR! " Error: 500 : vochain SendTx failed: createAccountTx: payload to and tx sender missmatch (ece1233c2173109ca8231b53dd1ed19e9615ec3f != ac336bcd5b6cad4ec6662a089fae4c7a17587653)

I guess they're related to the latest changes in their service:

0.4.0 - 2023-10-10

[BREAKING] New signatures for chain transactions.

So, @microstudi can you review this, merge and deploy to your server?

I understand that merging a breaking CI its not kosher, but as currently we're using an ancient version (3 months and 9 versions ago), currently is broken too. At least by my local tries, I've seen that it works sometimes 😄

@andreslucena andreslucena marked this pull request as ready for review October 11, 2023 08:14
@andreslucena
Copy link
Collaborator Author

I understand that merging a breaking CI its not kosher, but as currently we're using an ancient version (3 months and 9 versions ago), currently is broken too. At least by my local tries, I've seen that it works sometimes 😄

Vocdoni has mentioned thats because the v0.4.0 deploy isn't in staging yet, so I tried to v0.3.2 as that'd be more stable, but I still see that we have a failing spec... So as I say before, lets merge this and deploy it as its less broken than what we have now 🙈

@@ -20,7 +20,6 @@ group :development, :test do
end

group :development do
gem "i18n-tasks", "~> 0.9.37"
Copy link
Contributor

Choose a reason for hiding this comment

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

yes

Copy link
Contributor

@microstudi microstudi left a comment

Choose a reason for hiding this comment

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

we need to check the tests!

@andreslucena
Copy link
Collaborator Author

@microstudi last time I talked with the people from Vocdoni, they told me that the failing spec is related to how they handle their versions and APIs while they're on development. Basically, for now:

v0.4.0 - works with the dev environment
v0.3.x - works with the stag environment

In a few days/weeks they'll migrate stag to be compatible with the v0.4.0 API. Until that time we have a couple options:

  1. To change our environment to dev
  2. To upgrade to v0.3.x instead of v0.4.0
  3. To wait a couple days/weeks until stag is compatible with v0.4.0

As we don't have any current development in our side on the near future, I think it's better for now to just wait (3rd option) and this start blocking us then we can discuss the other options.

For the moment I sent a rerun to see if the stag/0.4.0 thing already happened 🤞🏽

Currently the Vochain isn't compatible with v0.4.0, so we'll
change our SDK configuration to development temporarily.

Once v0.4.0 is compatible with staging we will turn back to that
environment, as it's more stable than development.
@andreslucena
Copy link
Collaborator Author

Closing this as it was superseded by #72

@microstudi microstudi deleted the deps/vocdoni-sdk-0.3.1 branch May 22, 2024 15:13
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.

2 participants