Skip to content

Conversation

@Itay-Tsabary-Starkware
Copy link
Collaborator

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor

@TzahiTaub TzahiTaub left a comment

Choose a reason for hiding this comment

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

@TzahiTaub reviewed 13 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Itay-Tsabary-Starkware).


crates/apollo_http_server/src/http_server.rs line 72 at r1 (raw file):

        // Parses the bind address from HttpServerConfig, returning an error for invalid addresses.
        let HttpServerStaticConfig { ip, port } = self.config.static_config;

If you already made a function and everything

Suggestion:

let (ip, port) = self.config.ip_and_port();

Copy link
Collaborator

@matanl-starkware matanl-starkware left a comment

Choose a reason for hiding this comment

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

@matanl-starkware reviewed 13 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Itay-Tsabary-Starkware).


crates/apollo_http_server_config/src/config.rs line 36 at r1 (raw file):

    }

    pub fn ip_and_port(&self) -> (IpAddr, u16) {

Consider renaming and/or change the return value

Suggestion (i):

endpoint

Code snippet (ii):

pub fn address(&self) -> SocketAddr

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the 01-05-apollo_http_server_config_split_config_to_dynamic_and_static branch from 3093d02 to 93ae2b1 Compare January 7, 2026 07:02
Copy link
Collaborator Author

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware left a comment

Choose a reason for hiding this comment

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

@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 12 of 13 files reviewed, 1 unresolved discussion (waiting on @matanl-starkware and @TzahiTaub).


crates/apollo_http_server/src/http_server.rs line 72 at r1 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

If you already made a function and everything

Right, missed this one, fixed 🙏

Copy link
Collaborator Author

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware left a comment

Choose a reason for hiding this comment

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

@Itay-Tsabary-Starkware resolved 1 discussion.
Reviewable status: 12 of 13 files reviewed, all discussions resolved (waiting on @matanl-starkware and @TzahiTaub).

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the 01-05-apollo_http_server_config_split_config_to_dynamic_and_static branch 2 times, most recently from 70acfc6 to bdae043 Compare January 7, 2026 08:05
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware changed the base branch from main-v0.14.1-committer to graphite-base/11422 January 7, 2026 08:39
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the 01-05-apollo_http_server_config_split_config_to_dynamic_and_static branch from bdae043 to 33c5a71 Compare January 7, 2026 08:39
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware changed the base branch from graphite-base/11422 to 01-07-deployment_move_config_value_to_an_appropriate_yaml_file January 7, 2026 08:39
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the 01-05-apollo_http_server_config_split_config_to_dynamic_and_static branch from 33c5a71 to a2c02f2 Compare January 7, 2026 08:39
@graphite-app graphite-app bot changed the base branch from 01-07-deployment_move_config_value_to_an_appropriate_yaml_file to main-v0.14.1-committer January 7, 2026 08:40
@graphite-app
Copy link

graphite-app bot commented Jan 7, 2026

Merge activity

  • Jan 7, 8:40 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the 01-05-apollo_http_server_config_split_config_to_dynamic_and_static branch from a2c02f2 to e4e0c77 Compare January 7, 2026 09:28
Copy link
Collaborator Author

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware left a comment

Choose a reason for hiding this comment

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

@Itay-Tsabary-Starkware reviewed 16 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware).

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware added this pull request to the merge queue Jan 7, 2026
Merged via the queue into main-v0.14.1-committer with commit f9d0ef6 Jan 7, 2026
33 of 35 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants