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

refactor!: code optimization as stricter linters are in place #188

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

HenryQW
Copy link
Contributor

@HenryQW HenryQW commented Apr 19, 2024

make dev miserable again

@HenryQW HenryQW marked this pull request as draft April 19, 2024 03:54
}

// should not include genesis account
if nodeInfo.Account == ethereum.AddressGenesis {
Copy link
Contributor Author

@HenryQW HenryQW May 1, 2024

Choose a reason for hiding this comment

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

I am not sure why we should check AddressGenesis here?

@@ -24,7 +25,7 @@ import (
"go.uber.org/zap"
)

var (
var ( // FIXME: Name should be "operation_pool", update the file naem too.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am keeping the file name unchanged so we can have a diff view. It eventually needs to be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same for the db table name, search for // FIXME

@HenryQW HenryQW changed the title chore: add strict linters refactor!: code optimization as stricter linters are in place May 1, 2024
if nodeInfo.Account == ethereum.AddressGenesis {
return nil
}

mutex.Lock()
defer mutex.Unlock()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not think mutex is required here, we can just use the range index to avoid race conditions.

}
}

return nil
}

func (s *server) getNodesFromDB(ctx context.Context) ([]*schema.Node, error) {
nodes, err := s.databaseClient.FindNodes(ctx, schema.FindNodesQuery{})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when the number of nodes increases, lacking paginations will cause problems.

@HenryQW HenryQW requested review from polebug and brucexc May 1, 2024 22:10
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.

None yet

1 participant