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

Documentation revisions from Kostas. #100

Merged
merged 5 commits into from
Apr 17, 2022
Merged

Documentation revisions from Kostas. #100

merged 5 commits into from
Apr 17, 2022

Conversation

rpgoldman
Copy link
Contributor

Needs review followed by merge to master.

@rpgoldman
Copy link
Contributor Author

@ko56 -- would you please add a comment that briefly summarizes the changes you've made? It looks like you have revised the treatment of citations, and moved some introductory material above some more obscure material, but it's hard to get an overall picture from groveling over the Texinfo source. So a bullet list of change descriptions would be helpful.

@ko56
Copy link

ko56 commented Apr 14, 2022

Almost all my changes w.r.t. the latest in issue 76 are about formatting.
From your comment, it seems to me that whatever your version is now, it is out of sync with the latest in issue #76. E.g. the movement of material was discussed there on August 8 2021, and I submitted a patch on August 12 2021 using the "git way", as far as I could tell. Did you look at that?
The citations were revised/changed by you, in some version I got from git, probably after August 2021, but now I forgot exactly how I did that.
Unfortunately, git is very confusing for me.

They were being typeset as if they were text, instead of in a
list-like format.
@rpgoldman
Copy link
Contributor Author

rpgoldman commented Apr 15, 2022

Almost all my changes w.r.t. the latest in issue 76 are about formatting. From your comment, it seems to me that whatever your version is now, it is out of sync with the latest in issue #76. E.g. the movement of material was discussed there on August 8 2021, and I submitted a patch on August 12 2021 using the "git way", as far as I could tell. Did you look at that? The citations were revised/changed by you, in some version I got from git, probably after August 2021, but now I forgot exactly how I did that. Unfortunately, git is very confusing for me.

Got it. I am doing a pass over this as a review, and will later make the changes. Going forward, please try to keep your editor from changing whitespace. There are a bunch of blocks in the diff that I have to read carefully to see whether there are textual changes or only that the text has been re-filled.

(FWIW, I made this problem worse by committing a version with trailing whitespace removed because of my git configuration...)

@ko56
Copy link

ko56 commented Apr 15, 2022

Sorry, I like to have Emacs fill my text, not realizing that it would cause you a problem.

@rpgoldman
Copy link
Contributor Author

Sorry, I like to have Emacs fill my text, not realizing that it would cause you a problem.

That's OK; I do, too. I have found it is often helpful to do the re-filling once up front, commit that, and then edit from there. That gives one diff to ignore and one to review.

It's not a huge problem, but it makes things easier if we can manage it.

Copy link
Contributor Author

@rpgoldman rpgoldman left a comment

Choose a reason for hiding this comment

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

Need to clear these before merging.

shop3/docs/manual.texinfo Outdated Show resolved Hide resolved
shop3/docs/manual.texinfo Outdated Show resolved Hide resolved
shop3/docs/manual.texinfo Outdated Show resolved Hide resolved
shop3/docs/manual.texinfo Outdated Show resolved Hide resolved
shop3/docs/manual.texinfo Outdated Show resolved Hide resolved
shop3/docs/manual.texinfo Outdated Show resolved Hide resolved
shop3/docs/manual.texinfo Outdated Show resolved Hide resolved
shop3/docs/manual.texinfo Show resolved Hide resolved
shop3/docs/manual.texinfo Outdated Show resolved Hide resolved
shop3/docs/manual.texinfo Show resolved Hide resolved
@rpgoldman
Copy link
Contributor Author

Note: should fix the docstring for find-plans in SHOP3, or fix how it is translated to Texinfo: the list is poorly formatted in the documentation.

@ko56
Copy link

ko56 commented Apr 15, 2022

I agree, I didn't mention this issue because I didn't know what to do about it
#100 (comment)

@rpgoldman
Copy link
Contributor Author

I agree, I didn't mention this issue because I didn't know what to do about it #100 (comment)

Added new issue #101 -- I can work on that in a different PR

@ko56
Copy link

ko56 commented Apr 17, 2022

I'm not sure how we can sync up now. How do I get the latest from you?

@rpgoldman
Copy link
Contributor Author

I'm not sure how we can sync up now. How do I get the latest from you?

I will try to get this all edited together and merged today or tomorrow. Then we can resync, after the merge to main.

On the Mac, there is an ancient and incompatible version in `/usr/bin` and I needed the ability to use the one in `/opt/homebrew`. Now the one in your path can be overridden by setting an environment variable.
@rpgoldman rpgoldman marked this pull request as ready for review April 17, 2022 15:52
@rpgoldman
Copy link
Contributor Author

I will merge this after tests run. Need to update the online documentation pages, too.

@rpgoldman rpgoldman merged commit d6e04e1 into master Apr 17, 2022
@rpgoldman rpgoldman deleted the ko-doc-mods branch April 17, 2022 16:59
@ko56
Copy link

ko56 commented Apr 25, 2022

I obtained all your changes by git pull origin master, and I now have two more items:

  1. I fixed some occurrences of @code{} which should have been @var{}. Compressed file is here:
    manual.texinfo.gz

  2. I suggest removing sec. 3.7 Hook Routines, as I believe we agreed that it is obsolete w.r.t. SHOP3.

And one last thing: did you see my proposal in "Add alternative syntax for methods #72"?

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