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

README: Add simple usage #125

Merged
merged 1 commit into from
Dec 11, 2023
Merged

README: Add simple usage #125

merged 1 commit into from
Dec 11, 2023

Conversation

ldoktor
Copy link
Contributor

@ldoktor ldoktor commented Dec 7, 2023

let's add a very rough but practical usage guide to our README.

@ldoktor
Copy link
Contributor Author

ldoktor commented Dec 8, 2023

Rebased on top of #126 the README: Add simple usage is the commit intended for this PR

@ldoktor
Copy link
Contributor Author

ldoktor commented Dec 8, 2023

@pevogam and if you have some time, what would you say about this (only the last commit, I keep neglecting the documentation, this could IMO help the newcomers).

Copy link
Contributor

@pevogam pevogam left a comment

Choose a reason for hiding this comment

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

Hi @ldoktor overall a great improvement of usability for new users, I think we can just slightly improve on the advertising part and the section structure of the README.

False
>>> session.is_responsive()
False
Copy link
Contributor

Choose a reason for hiding this comment

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

I love this idea - it is compact, goes straight to the point, and offers some of the highest information per lines read.

`sftp` and others although it can be useful for pure streams as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the main advantage aexpect has over pexpect? Because from the first sentence it sounds almost like "this is yet another way of doing what you can already do there". It would be great to focus on what aexpect can do and pexpect right from the very start. I suggest a first sentence starting with "This project provides services similar to ... but enhanced them with ..." Note I used "project" instead of "module" since some of the remote code running (remote door with remote decorators, remote objects, and remote control files) and other additional functionality comes to mind as a definite advantage that I am not sure pexpect has.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really a marketing person so I just wanted to add that it might be useful for non-bash environments as well. Improvements to this as well as the code below (remote execution) would be definitely welcome.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not trying to request a sales pitch but merely to have one initial sentence that describes in short what the project is about and what separates it from other similar projects (why it exists on the first place). So IIUC you say there isn't much more that aexpect can do than pexpect? I thought the project was started because there was something Red Hat or others needed that pexpect did not already provide.

False

Using this even for purely bash-like constructs is good as you can leverage
Copy link
Contributor

Choose a reason for hiding this comment

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

This is great additional information and useful to have in the documentation but if it is in the README how about separating it in a second section called "Debugging"? The first one could be called "Overview" or "Summary".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean just a section or a separate page? Currently the whole readme is quite short so having it in a single page seems reasonable to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just a section in markdown to separate the two parts of "Debugging" and "Overview" (the upper part).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, makes sense

@@ -503,6 +503,7 @@ def find_unused_port(session, start_port=10024):
:returns: unused port that was found
:rtype: int
"""
# pylint: disable=C0301
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be great to rebase this branch on top of the main branch once #126 is merged.

@ldoktor
Copy link
Contributor Author

ldoktor commented Dec 11, 2023

How about this, rebased and added some hints on extra features.

Copy link
Contributor

@pevogam pevogam left a comment

Choose a reason for hiding this comment

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

Looks great, just a few more minor comments on typos and I think this is ready.

README.rst Outdated
@@ -3,4 +3,78 @@ Aexpect: Control your interactive applications

This module provides services similar to the `pexpect` python library,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we phrase this as "this project"? After all, this is documentation for the entire project and aexpect is not a single module for quite a while now if since the very beginning (having more than just client module).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

README.rst Outdated
`sftp` and others although it can be useful for pure streams as well.

It's enhanced of multi-pattern matching, convenience features for
Copy link
Contributor

Choose a reason for hiding this comment

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

"It's enhanced with" or if you want "It also offers" / "It also has the capabilities of"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

README.rst Outdated
extra output from time to time (eg. kernel messages) over the output.

There are also extra classes to simplify executing parts of program
Copy link
Contributor

Choose a reason for hiding this comment

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

"parts of a program"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack


Simple usage
------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea about this, another alternative would be "Quick start" but both work well.

let's add a very rough but practical usage guide to our README.

Signed-off-by: Lukáš Doktor <[email protected]>
Copy link
Contributor

@pevogam pevogam left a comment

Choose a reason for hiding this comment

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

LGTM

@ldoktor ldoktor merged commit b7babba into avocado-framework:main Dec 11, 2023
3 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.

2 participants