Compute ctl main refactor startup context #7600
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A couple lines moved further down in main(), and one case of using Option<&str> instead of Option<&String>.
This commit is intentionally designed to have as small a diff as possible. To that end, the basic idea is that each distinct "chunk" of the previous main() has been wrapped in its own function, with the return values from each function being passed directly into the next. The structure of main() is now visible from its contents: 1. init() 2. process_cli() 3. wait_spec() 4. start_postgres() 5. wait_postgres() 6. cleanup_and_exit() There's a lot of other work that can / should(?) be done beyond this, but I figure that's more opinionated, and this should be a solid start.
hlinnaka
changed the base branch from
main
to
sharnoff/compute_ctl-main()-phase-refactor
2862 tests run: 2739 passed, 2 failed, 121 skipped (full report)Failures on Postgres 16Test coverage report is not availableThe comment gets automatically updated with the latest test results
a77dd07 at 2024-05-03T07:16:02.373Z :recycle: |
sharnoff
added a commit
that referenced
this pull request
May 4, 2024
This is part 1 of 2 for applying the changes recommended in #7600, keeping the diff minimal by breaking process_cli into three pieces so we can extract startup_context_from_env() from the middle. This will be squashed into the prior commit before merging. Kept it separate for now, just to make the changes easier to review. Co-authored-by: Heikki Linnakangas <heikki@neon.tech>
sharnoff
added a commit
that referenced
this pull request
May 4, 2024
Part of applying the changes from #7600. This piece *technically* can change the semantics because now the context guard is held before process_cli, but... the difference is likely quite small. Co-authored-by: Heikki Linnakangas <heikki@neon.tech>
sharnoff
added a commit
that referenced
this pull request
May 4, 2024
This is part 1 of 2 for applying the changes recommended in #7600, keeping the diff minimal by breaking process_cli into three pieces so we can extract startup_context_from_env() from the middle. This will be squashed into the prior commit before merging. Kept it separate for now, just to make the changes easier to review. Co-authored-by: Heikki Linnakangas <heikki@neon.tech>
sharnoff
force-pushed
the
sharnoff/compute_ctl-main()-phase-refactor
branch
from
ffe65ef to
10a0754
Compare
sharnoff
added a commit
that referenced
this pull request
May 4, 2024
Part of applying the changes from #7600. This piece *technically* can change the semantics because now the context guard is held before process_cli, but... the difference is likely quite small. Co-authored-by: Heikki Linnakangas <heikki@neon.tech>
|
These changes were included in PR #7577 |
sharnoff
added a commit
that referenced
this pull request
May 4, 2024
Part of applying the changes from #7600. This piece *technically* can change the semantics because now the context guard is held before process_cli, but... the difference is likely quite small. Co-authored-by: Heikki Linnakangas <heikki@neon.tech>
sharnoff
added a commit
that referenced
this pull request
May 7, 2024
Part of applying the changes from #7600. This piece *technically* can change the semantics because now the context guard is held before process_cli, but... the difference is likely quite small. Co-authored-by: Heikki Linnakangas <heikki@neon.tech>
sharnoff
added a commit
that referenced
this pull request
May 7, 2024
Part of applying the changes from #7600. This piece *technically* can change the semantics because now the context guard is held before process_cli, but... the difference is likely quite small. Co-authored-by: Heikki Linnakangas <heikki@neon.tech>
conradludgate
pushed a commit
that referenced
this pull request
May 8, 2024
Part of applying the changes from #7600. This piece *technically* can change the semantics because now the context guard is held before process_cli, but... the difference is likely quite small. Co-authored-by: Heikki Linnakangas <heikki@neon.tech>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A little more refactoring for PR #7577, see comment #7577 (comment)