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.

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

tools/unitctl: Elaborate on features in unitctl readme. #1359

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

avahahn
Copy link
Contributor

@avahahn avahahn commented Jul 15, 2024

I wanted the unitctl readme to elaborate on the how and the limitations of each function that the tool offers... not just a one-line description and an example. I added more text to each section. Please leave feedback if you feel there are any significant details left out.

Signed-off-by: Ava Hahn <a.hahn@f5.com>
Copy link
Contributor

@javorszky javorszky left a comment

Choose a reason for hiding this comment

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

This is really cool! A few minor changes, but otherwise looks good!

tools/unitctl/README.md Outdated Show resolved Hide resolved
tools/unitctl/README.md Show resolved Hide resolved
tools/unitctl/README.md Show resolved Hide resolved
tools/unitctl/README.md Show resolved Hide resolved
tools/unitctl/README.md Show resolved Hide resolved
tools/unitctl/README.md Outdated Show resolved Hide resolved
tools/unitctl/README.md Show resolved Hide resolved
tools/unitctl/README.md Show resolved Hide resolved
tools/unitctl/README.md Show resolved Hide resolved
tools/unitctl/README.md Show resolved Hide resolved
@callahad
Copy link
Collaborator

My review would mirror @javorszky's - otherwise lgtm

Copy link
Member

@ac000 ac000 left a comment

Choose a reason for hiding this comment

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

Apply suggestions from code review

Co-authored-by: Gabor Javorszky <gabor@javorszky.co.uk>

I'm not sure this warrants a Co-authored-by. Reviewers wouldn't normally expect to give a Co-developed-by tag, unless maybe they suggested significant changes.

Besides, the tag we use for this is Co-developed-by (and such a tag should be immediately followed by a Signed-off-by)

This patch should of course be merged with the first one...

Copy link
Contributor

@javorszky javorszky left a comment

Choose a reason for hiding this comment

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

UNIT -> Unit, all changes are suggestions

tools/unitctl/README.md Outdated Show resolved Hide resolved
tools/unitctl/README.md Outdated Show resolved Hide resolved
tools/unitctl/README.md Outdated Show resolved Hide resolved
tools/unitctl/README.md Outdated Show resolved Hide resolved
Signed-off-by: Ava Hahn <a.hahn@f5.com>
Copy link

@dekobon dekobon left a comment

Choose a reason for hiding this comment

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

This looks great!

tools/unitctl/README.md Show resolved Hide resolved
@avahahn avahahn requested a review from dekobon July 29, 2024 15:22
@avahahn avahahn merged commit 1b48430 into nginx:master Jul 29, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants