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

WIP feat: make messenger and protocol inits standalone #3835

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

moul
Copy link
Member

@moul moul commented Dec 30, 2021

  • delete OpenAccount (sync) and rename OpenAccountWithProgress into OpenAccount
  • delete CloseAccount (sync) and rename CloseAccountWithProgress into CloseAccount
  • extract the protocol init from OpenAccount into a new OpenProtocol endpoint
  • extract the protocol init from CloseAccount into a new CloseProtocol endpoint
  • move the NetworkConfig message definition from BertyAccounts to BertyMessenger
  • add new errcode.ErrCodes to manage new cases
  • update JS code

—-

Note: I considered moving all the NetworkConfig* stuff from BertyAccount to BertyMessenger, but the changes were too big and risky, I prefer to let the JS team refactor this part later, ideally when refactoring the onboarding

My suggestion -> create/import/open account are instant, then, you load the config and decide if you want to ask new prompts (i.e., BLE permission etc), then only you call OpenProtocol with the appropriate configuration

@moul moul self-assigned this Dec 30, 2021
@moul moul changed the title feat: make messenger and protocol inits standalone WIP feat: make messenger and protocol inits standalone Dec 30, 2021
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Signed-off-by: Manfred Touron <[email protected]>
Comment on lines -115 to 120
func NewService(opts *Options) (_ Service, err error) {
func NewService(opts *Options) (Service, error) {
rootCtx, rootCancelCtx := context.WithCancel(context.Background())

var s *service
var (
s *service
err error
)
endSection := tyber.SimpleSection(rootCtx, opts.Logger, "Initializing AccountService")
defer func() { endSection(err, tyber.WithDetail("RootDir", s.rootdir)) }()
Copy link
Contributor

@n0izn0iz n0izn0iz Mar 7, 2022

Choose a reason for hiding this comment

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

removing the named return will prevent tyber from catching the correct error

with the named return, a return nil, someErr will set the named return to someErr before executing the defers

that's also why we need to wrap the endSection call in a lambda

@jefft0 jefft0 requested review from n0izn0iz and jefft0 and removed request for n0izn0iz and jefft0 April 21, 2023 06:03
@jefft0 jefft0 removed the 🚧 WIP label Apr 21, 2023
@jefft0 jefft0 unassigned moul Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants