-
Notifications
You must be signed in to change notification settings - Fork 244
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
Chore: Refactor_wallet_manager #1667
Conversation
✅ Deploy Preview for specter-desktop-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
The experimental session-feature used in the |
This is ready for review. I'd like to get this in part by part. I could continue here but i think it's better to merge it as is. |
describe('Test the actions in UTXO list', () => { | ||
// See https://github.com/cypress-io/cypress/issues/21138 | ||
if (Cypress.env("CI")) { | ||
const login = (name,password) => { |
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.
Perhaps I am overlooking sth. but where is the name variable set?
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.
It's defining a function and the name is the parameter here. It's initialized with the arguments at call time, e.g. in line 39.
So this does not yet make the wallet manager session aware? It is just refactoring to use less instance variables such that when the wallet manager is session aware things like the |
This was just a side_effect of that change. The main point from my point of view is that the |
…sktop into refactor_wallet_manager
This is a preparation of making the
WalletManager
aware of the active node via a session-attribute.relates to #1661