Skip to content

Merge bridge / delivery / rest-server into one process#63

Merged
bladehan1 merged 2 commits intobttcprotocol:developfrom
ClarkChenc:merge_delivery_services
Oct 13, 2023
Merged

Merge bridge / delivery / rest-server into one process#63
bladehan1 merged 2 commits intobttcprotocol:developfrom
ClarkChenc:merge_delivery_services

Conversation

@ClarkChenc
Copy link
Copy Markdown
Contributor

This pr is aimed to start bridge / rest-server along with deliveryd process.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 10, 2023

Codecov Report

Merging #63 (e0f74e9) into develop (320b948) will not change coverage.
Report is 2 commits behind head on develop.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@           Coverage Diff            @@
##           develop      #63   +/-   ##
========================================
  Coverage    60.29%   60.29%           
========================================
  Files           52       52           
  Lines         4423     4423           
========================================
  Hits          2667     2667           
  Misses        1519     1519           
  Partials       237      237           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

📢 Have feedback on the report? Share it here.

Comment thread bridge/cmd/start.go
// exit
os.Exit(1)
for _, service := range services {
srv := service
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any reason to allocate a new variable here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No need to allocate new variable here. But for some cases, we need to cache objects' addresses in for-range scope, and service will always point to a same address, so we need to allocate a new variable. So such idiom can prevent mistakes in the future.

Comment thread cmd/deliveryd/main.go

// bridge flags = start flags (all, only) + root bridge cmd flags.
cmd.Flags().Bool("all", false, "start all bridge services")
cmd.Flags().StringSlice("only", []string{}, "comma separated bridge services to start")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are "all" and "only" used in some functions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

bridge has two service, listener and processor, "all" means to start all services, while "only" can start target service. In general, we use "all" when start bridge. Here is just to decorate main cmd just as bridge cmd.

Comment thread cmd/deliveryd/main.go Outdated
tracerWritePerm = 0666
pvKeyFilePerm = 0777
pvStateFilePerm = 0777
nodeDirPerm = 0o755
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do you need o permissions?

@bladehan1 bladehan1 merged commit 363c61b into bttcprotocol:develop Oct 13, 2023
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.

4 participants