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

Format mgr:pconn as YAML #1780

Closed
wants to merge 20 commits into from
Closed

Format mgr:pconn as YAML #1780

wants to merge 20 commits into from

Conversation

kinkie
Copy link
Contributor

@kinkie kinkie commented Apr 14, 2024 •
edited
Loading

Also rework to rely on PackableStream

@kinkie
Copy link
Contributor Author

kinkie commented Apr 14, 2024

TODO: use spaces() from PR #1735

@kinkie kinkie added the S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) label Apr 14, 2024
rousskov
rousskov requested changes Apr 14, 2024
View reviewed changes
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Thank you for this conversion.

src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
@kinkie
Copy link
Contributor Author

kinkie commented Apr 16, 2024

Should be ready for review

rousskov
rousskov requested changes Apr 16, 2024
View reviewed changes
src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.h Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Apr 16, 2024
rousskov
rousskov requested changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

I would not be surprised if this is the last review iteration (for me). Thank you.

src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.h Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
src/pconn.cc Outdated Show resolved Hide resolved
kinkie and others added 2 commits April 18, 2024 14:19
@kinkie @rousskov
Apply suggestions from code review
350ce5e 
Co-authored-by: Alex Rousskov <rousskov@measurement-factory.com>
@rousskov
fixup: Remove unwanted(?) trailing space in heading
62d3d88
rousskov
rousskov approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Approving after removing (hopefully unwanted) trailing whitespace from connection use histogram heading. Thank you.

@rousskov
Copy link
Contributor

TODO: use spaces() from PR #1735

FWIW, if you prefer, I am OK with landing this PR without spaces() if you do not want to wait for SMP support decision and associated issues (and promise to adjust this code as needed later). I am also OK with waiting (i.e. not merging this PR now). Your call.

squid-anubis pushed a commit that referenced this pull request Apr 19, 2024
@kinkie @squid-anubis
Format mgr:pconn as YAML (#1780)
2a580c1 
Also rework to rely on PackableStream
@squid-anubis squid-anubis added M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-passed-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels labels Apr 19, 2024
@kinkie
Copy link
Contributor Author

kinkie commented Apr 24, 2024

TODO: use spaces() from PR #1735

FWIW, if you prefer, I am OK with landing this PR without spaces() if you do not want to wait for SMP support decision and associated issues (and promise to adjust this code as needed later). I am also OK with waiting (i.e. not merging this PR now). Your call.

I'm landing this in order to keep the backlog short. I'll revisit this once PR 1735 lands

@kinkie kinkie added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-author author action is expected (and usually required) labels Apr 24, 2024
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-passed-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Apr 24, 2024
@yadij yadij removed the S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) label Apr 29, 2024
kinkie added a commit to kinkie/squid that referenced this pull request Jun 22, 2024
@kinkie
Format mgr:pconn as YAML (squid-cache#1780)
89e1fd4 
Also rework to rely on PackableStream
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rousskov rousskov rousskov approved these changes

Assignees
No one assigned
Labels
M-merged https://github.com/measurement-factory/anubis#pull-request-labels
Projects
None yet
Milestone
No milestone
4 participants
@kinkie @rousskov @yadij @squid-anubis