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

Added missing parameters to kiwix-serve.1 #626

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

iArchitSharma
Copy link
Contributor

@iArchitSharma iArchitSharma commented Jul 9, 2023

Fixes #609

@kelson42
Copy link
Contributor

kelson42 commented Jul 9, 2023

@iArchitSharma Thank you for your PR, we will do our best to review it quickly!

Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

I started reading the updated man page and wrote a couple of comments until I figured out that the content of the man page differs from the online documentation. Ideally there should be a shared source for documentation content (e.g. the man page is generated from the .rst file). If that is too much to ask for this PR, I think that at least the man page must contain the same text as the online documentation.

src/man/kiwix-serve.1 Outdated Show resolved Hide resolved
src/man/kiwix-serve.1 Outdated Show resolved Hide resolved
@iArchitSharma
Copy link
Contributor Author

@veloman-yunkan I have updated the manpage, is it good now?

Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

Please also include the link to the online documentation in the DOCUMENTATION section.

src/man/kiwix-serve.1 Outdated Show resolved Hide resolved
src/man/kiwix-serve.1 Show resolved Hide resolved
src/man/kiwix-serve.1 Show resolved Hide resolved
src/man/kiwix-serve.1 Show resolved Hide resolved
src/man/kiwix-serve.1 Outdated Show resolved Hide resolved
@iArchitSharma
Copy link
Contributor Author

@veloman-yunkan Done with everything you told!

Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

You misunderstood one of my previous comments. After you fix it please squash all your commits into one and the PR will be good to merge.

src/man/kiwix-serve.1 Outdated Show resolved Hide resolved
@iArchitSharma
Copy link
Contributor Author

@veloman-yunkan Is it good now?

@iArchitSharma
Copy link
Contributor Author

You misunderstood one of my previous comments. After you fix it please squash all your commits into one and the PR will be good to merge.

Can you tell me how I can squash all of my commits? I searched the web, but it didn't help.

@veloman-yunkan
Copy link
Collaborator

@veloman-yunkan Is it good now?

It is a little short of being good:

  1. There is no --library option in the first version of kiwix-serve invocation.
  2. The two versions of kiwix-serve invocation appear on a single line.

Screenshot from 2023-07-11 19-05-36

@veloman-yunkan
Copy link
Collaborator

Can you tell me how I can squash all of my commits? I searched the web, but it didn't help.

Use interactive git rebase

@veloman-yunkan
Copy link
Collaborator

veloman-yunkan commented Jul 11, 2023

@veloman-yunkan Is it good now?

It is a little short of being good:

  1. There is no --library option in the first version of kiwix-serve invocation.

  2. The two versions of kiwix-serve invocation appear on a single line.

Screenshot from 2023-07-11 19-05-36

Also LIBRARY_FILE_PATH and/or ZIM_FILE_PATH arguments are not optional. Why don't you literally reproduce the text found in the online documentation and/or my previous comment?

@iArchitSharma
Copy link
Contributor Author

@veloman-yunkan done!

@iArchitSharma
Copy link
Contributor Author

iArchitSharma commented Jul 12, 2023

Can you tell me how I can squash all of my commits? I searched the web, but it didn't help.

Use interactive git rebase

still not able to, can you merge without me stashing all the commits?

@kelson42
Copy link
Contributor

@iArchitSharma No, because this will pollute the git revision log. What is the problem? You don't use git on the command line?

@iArchitSharma
Copy link
Contributor Author

iArchitSharma commented Jul 12, 2023

@iArchitSharma No, because this will pollute the git revision log. What is the problem? You don't use git on the command line?

I use git but have never squashed before
this is the command i should be using right - git rebase -i <commit>
should i pass the first commit or last commit i made in <commit>

@iArchitSharma
Copy link
Contributor Author

Finally done with squash too!

@veloman-yunkan
Copy link
Collaborator

Finally done with squash too!

But the result is still not available on GitHub. Note that you have to provide the --force option to git push after you rewrite the commit history.

@veloman-yunkan
Copy link
Collaborator

veloman-yunkan commented Jul 12, 2023

@veloman-yunkan Is it good now?

It is a little short of being good:

  1. There is no --library option in the first version of kiwix-serve invocation.

  2. The two versions of kiwix-serve invocation appear on a single line.

Screenshot from 2023-07-11 19-05-36

Item 2 was not addressed.

@iArchitSharma
Copy link
Contributor Author

@veloman-yunkan Is it good now?

It is a little short of being good:

  1. There is no --library option in the first version of kiwix-serve invocation.
  2. The two versions of kiwix-serve invocation appear on a single line.

Screenshot from 2023-07-11 19-05-36

Item 2 was not addressed.

Done

@iArchitSharma
Copy link
Contributor Author

Finally done with squash too!

But the result is still not available on GitHub. Note that you have to provide the --force option to git push after you rewrite the commit history.

All done!

Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

Thanks for bearing with me!

@iArchitSharma
Copy link
Contributor Author

Thanks for bearing with me!

I really appreciate your assistance. I've learned a lot

@kelson42 kelson42 merged commit 7a7eaab into kiwix:main Jul 12, 2023
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.

kiwix-serve: update manpage
3 participants