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.

Sign up for GitHub

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

Jump to bottom

Improve cyclomatic complexity of func (s *ServerContext) Run(ctx context.Context, config *serverconfig.Config) #1575

Open
adriantam opened this issue Apr 26, 2024 · 2 comments
Open

Improve cyclomatic complexity of func (s *ServerContext) Run(ctx context.Context, config *serverconfig.Config) #1575

adriantam opened this issue Apr 26, 2024 · 2 comments
Labels
good first issue Good for newcomers refactor A cleanup is needed

Comments

@adriantam
Copy link
Member

adriantam commented Apr 26, 2024 •
edited

As per #1552 (comment), the function ServerContext Run()

openfga/cmd/run/run.go

Line 279 in abf5664

func (s *ServerContext) Run(ctx context.Context, config *serverconfig.Config) error {
is highly complex and require refactoring. The PR slight reduces complexity. However, we require more systematic approach to make the server config better
@adriantam adriantam added good first issue Good for newcomers refactor A cleanup is needed labels Apr 26, 2024
@00chorch
Copy link
Contributor

00chorch commented May 6, 2024

Hi @adriantam, I'm working on this one. I will send a PR today/tomorrow.

00chorch added a commit to 00chorch/openfga that referenced this issue May 6, 2024
@00chorch
refactor(cmd): reduce complexity of Server Run command
5beb40e 
CC: Cyclomatic Complexity
MI: Maintainability Index

(*ServerContext).Run function metrics:
- Before: CC=49, MI=7
- w/ Refactor: CC=26, MI=21

Resolves openfga#1575
@00chorch 00chorch mentioned this issue May 6, 2024
Closed
4 tasks
00chorch added a commit to 00chorch/openfga that referenced this issue May 15, 2024
@00chorch
refactor(cmd): reduce complexity of Server Run command
77a34ec 
refactored some of the logic to sub-functions
- buildServerOpts
- dialGrpc

Partial work for ticket openfga#1575
@00chorch
Copy link
Contributor

Hi @adriantam ,
as the merge was messy to compare/fix, I've reset local branch to match upstream/main and will re-submit the changes for review. This caused the PR #1605 to get closed. Can you please re-open it for me.

My plan is to:

  1. reopen in draft mode and review 2 out of the 4 func refactors in run.go file
  2. if ok, proceed to the last two refactors
  3. move from draft to formal PR review

The stepped approach is
a) reduce lines to review per iteration
b) avoid (or minimize) dealing with conflicts during merges with upstream

Thanks for your support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers refactor A cleanup is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor(cmd): reduce complexity of Server Run command 00chorch/openfga
2 participants
@adriantam @00chorch