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

Tweak 0.3 docs #79

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Conversation

richb-hanover
Copy link
Contributor

Update screen shots to 0.3 version;
Update text to match;
Create UNDERSTANDING.md page;
Factor command line info from README into COMMANDLINE.md file;
Add batch_add_border.sh script for simple screen-shot processing;

COMMANDLINE.md Outdated Show resolved Hide resolved
COMMANDLINE.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
UNDERSTANDING.md Outdated Show resolved Hide resolved
UNDERSTANDING.md Outdated Show resolved Hide resolved
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe just do a local test to avoid the odd sensoring?

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 happen to be at my Mom's house today, where the network is fast but has high latency. I think these charts are better - showing typical results, not the "best possible" when using a good router.

Copy link
Owner

Choose a reason for hiding this comment

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

Well the client screenshot doesn't have to match the result. You could also save and add the result to the data folder.

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 think the files are good now.

Copy link
Owner

Choose a reason for hiding this comment

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

You haven't changed them?

Copy link
Owner

Choose a reason for hiding this comment

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

Same here

CHANGELOG.md Outdated Show resolved Hide resolved
CLI.md Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

It probably makes sense to do these changes in the code too.

README.md Outdated Show resolved Hide resolved
UNDERSTANDING.md Outdated Show resolved Hide resolved
@richb-hanover
Copy link
Contributor Author

[I'm going off-grid for about 8 or 9 hours. These are good comments: keep sending them. I will address them when I return. Thanks]

@richb-hanover
Copy link
Contributor Author

Any further changes necessary? Thanks again

RESULTS.md Outdated Show resolved Hide resolved
RESULTS.md Outdated Show resolved Hide resolved
RESULTS.md Outdated Show resolved Hide resolved
@richb-hanover
Copy link
Contributor Author

@Zoxc Thanks for updating the README. Now that I have returned home (yesterday was a travel day), I made those edits. I think this is looking pretty good. Congratulations!

@richb-hanover
Copy link
Contributor Author

@Zoxc Good catch. Fixed

@richb-hanover
Copy link
Contributor Author

@Zoxc - I have merged master back into this branch. Are there any other outstanding changes you need? Thanks

@Zoxc
Copy link
Owner

Zoxc commented Sep 23, 2024

You don't need to create merge commit when there's no conflicts.

I do think screenshots without any censoring would be better.

@richb-hanover
Copy link
Contributor Author

@Zoxc OK. Do you see any other changes? Thanks.

CLI.md Outdated Show resolved Hide resolved
Changed `./crusader ...` to `crusader ...`
@richb-hanover
Copy link
Contributor Author

@Zoxc I see a merge conflict. Would you fix it? I cannot. Thanks

@Zoxc
Copy link
Owner

Zoxc commented Sep 24, 2024

I had new screenshots in mind instead of further editing.

@richb-hanover
Copy link
Contributor Author

richb-hanover commented Sep 24, 2024

I'm not really sure what changes to the screen shots you would like to see. Perhaps you could commit the current state, then substitute your own screen shots...

@richb-hanover
Copy link
Contributor Author

@Zoxc It has been a week since I made my last changes to the documentation. Is there a reason that you have not published them? (I have held off from publicizing Crusader until the documentation matches the software...) Thanks.

@@ -12,7 +12,8 @@ in the presence of upload and download traffic.
It also incorporates a continuous latency tester for
monitoring background responsiveness.

Crusader uses TCP and UDP ports 35481 (only) for its tests.
Copy link
Owner

Choose a reason for hiding this comment

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

I'd probably keep this and add something like:
Traffic generation uses TCP while UDP is used for latency and packet loss measurements.

@Zoxc
Copy link
Owner

Zoxc commented Oct 3, 2024

I'm not really sure what changes to the screen shots you would like to see.

I just wanted fresh new screenshots so they exactly match the actual application.

@richb-hanover
Copy link
Contributor Author

I am feeling stuck:

  • I believe it's important that the screen shots show real-world tests with less than perfect performance
  • I am not willing to display my personal test server's address in the screen shots.
  • It's enough work to prepare screen shots that I'm not willing to revise continually.

What do you suggest? I would be completely content for you to commit this PR, then make your own editorial changes/substitute screen shots. Would that work? Thank you.

Create a LOCAL_TESTS.md file;
Passes various markdown tests
@richb-hanover
Copy link
Contributor Author

@Zoxc I re-organized the repo to collect all the separate instruction files into a docs directory. I also created a "Running Local Tests" document, because I think this is an important use of Crusader.

What needs to happen to move this PR along? Thank you

@richb-hanover
Copy link
Contributor Author

@Zoxc I continue to use Crusader, and recommend it in my blog and on various forums (example: Reddit)

In particular, I would like to link to docs/LOCAL_TESTS.md for testing Wifi. But it hasn't yet been merged into the main repo.

What needs to be done to make this PR (#79) suitable for merging? Thanks.

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