Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Reduce cognitive complexity in main #1932

Merged
merged 5 commits into from
May 27, 2021

Conversation

ObsidianMinor
Copy link
Contributor

This replaces our massive clap App with a much simpler Cli struct that
has all the same information in an easier to understand and modify
format (types). Because everything is now typed (and because we've moved
clap::App creation out of main), we can further reduce the cognitive
complexity of main by removing lots of parameter checking and parsing,
removing that pesky clippy lint attribute.

Also it removes emoji in all the help messages. But they don't always work
anyway (Windows) and prevent us from doing this necessary refactor,
so meh.

@ObsidianMinor ObsidianMinor requested a review from a team as a code owner May 25, 2021 06:49
This replaces our massive clap App with a much simpler Cli struct that
has all the same information in an easier to understand and modify
format (types). Because everything is now typed (and because we've moved
clap::App creation out of main), we can further reduce the cognitive
complexity of main by removing lots of parameter checking and parsing,
removing that pesky clippy lint attribute.
@ObsidianMinor
Copy link
Contributor Author

I'm currently going over and verifying the API hasn't changed at all.

src/main.rs Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
@nilslice nilslice added interface Issues or improvements to the interface refactor labels May 26, 2021
src/main.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@nilslice nilslice left a comment

Choose a reason for hiding this comment

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

LGTM - each subcommand works as expected and I didn't catch any regressions, nice work!

Just left some minor changes, and q's on what we probably ought to move outside main.

@nilslice
Copy link
Contributor

I'm currently going over and verifying the API hasn't changed at all.

@ObsidianMinor, can you confirm you saw no changes?

Copy link
Contributor

@nilslice nilslice left a comment

Choose a reason for hiding this comment

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

Ok, confirmed offline with @ObsidianMinor that the API is consistent, and have observed the same with my testing. ✅

@nilslice nilslice merged commit d5cb868 into cloudflare:master May 27, 2021
@ObsidianMinor ObsidianMinor deleted the rfctr/main branch June 7, 2021 17:30
kflict referenced this pull request in CollectiveSynthesis2021/ORGIN2 Jun 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
interface Issues or improvements to the interface refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants