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

Remove install-tools.sh and documentation mentioning it #1305

Merged
merged 3 commits into from
Jun 3, 2017

Conversation

fingolfin
Copy link
Member

It only exists for legacy users, and those already know about install-tools.sh resp. have it. We should not avoid current users by mentioning it in the manual either.

This replaces #1125.

Please do not merge before @alex-konovalov had a chance to comment.

@codecov
Copy link

codecov bot commented May 7, 2017

Codecov Report

Merging #1305 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1305   +/-   ##
=======================================
  Coverage   61.22%   61.22%           
=======================================
  Files        1035     1035           
  Lines      356429   356429           
  Branches    14226    14226           
=======================================
  Hits       218229   218229           
- Misses     134625   134627    +2     
+ Partials     3575     3573    -2
Impacted Files Coverage Δ
src/funcs.c 63.68% <0%> (-0.42%) ⬇️
src/objset.c 41.14% <0%> (-0.29%) ⬇️
src/hpc/threadapi.c 31.25% <0%> (+0.39%) ⬆️

@markuspf
Copy link
Member

@alex-konovalov could you take a second to comment on this?

Copy link
Member

@olexandr-konovalov olexandr-konovalov left a comment

Choose a reason for hiding this comment

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

Thank you @fingolfin. I will be happy to adjust release wrapping scripts when this PR will be finished. Besides the changes that you're suggesting, there is also the file etc/README.tools which refers to tools.tar.gz archive. The file could possibly stay (it describes tools for package authors, included in the GAP distribution) but it should not mention tools.tar.gz. Instead, the first sentence could be "The GAP distribution includes some files that have mainly been provided to assist
package authors" (also replace pdf by text in line 6, remove referring to "archive" in line 7, and update or remove October 2011 at the end of etc/README.tools .

<C>etc/tools.tar.gz</C> which should be unpacked using the script
<F>etc/install-tools.sh</F>), the first non-empty line of
<F>manual.six</F> must be of the form
The first non-empty line of <F>manual.six</F> should be of the form
Copy link
Member

Choose a reason for hiding this comment

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

So, we throw away mentioning of doc/gapmacrodoc.pdf completely (cf. @fingolfin's comment in #1125). What to do with doc/gapmacrodoc.tex itself - does it stay where it is? Do we need to build PDF out of it during the release? I would leave tex for now, but do not build PDF.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed.

<P/>
If such a line is missing then it is assumed that the <F>manual.six</F> file
complies with the <F>gapmacro.tex</F> documentation format which is the
<C>default</C> format, and reading the file is delegated to
Copy link
Member

Choose a reason for hiding this comment

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

Last time (cd. #1125) I've misinterpreted default as a string and assumed that it means "used by default" . To avoid such misunderstandings, what about

If such a line is missing then it is assumed that the <F>manual.six</F> file
complies with the <F>gapmacro.tex</F> documentation, and reading this file is delegated to 
<C>HELP_BOOK_HANDLER.default.ReadSix</C>.

Copy link
Member Author

Choose a reason for hiding this comment

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

But I have not written that text. It was there before, it just got rewrapped. I have now update this PR to make that crystal clear (loosing the reformatting of the manual I applied before).

@fingolfin
Copy link
Member Author

I don't really agree with the new intro text to README.tools you suggest -- it could easily be misinterpreted as saying that authors of new packages should use these files.

I'd really prefer to simply remove README.tools. Those people who want to use those tools already know them. And those who don't know about them, well, I'd prefer if it stayed that way, and would rather steer them towards GAPDoc or AutoDoc.

@@ -109,6 +103,8 @@ no further regulations for the format of the <F>manual.six</F> file, other tha
what you yourself impose. If such a line is missing then it is assumed that
the <F>manual.six</F> file complies with the <F>gapmacro.tex</F> documentation format
which is the <C>default</C> format.
Copy link
Member

@olexandr-konovalov olexandr-konovalov May 31, 2017

Choose a reason for hiding this comment

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

I still don't like the <C>default</C> format - is is possible to rephrase that as I've already suggested?

@olexandr-konovalov
Copy link
Member

OK - changes approved, and I agree that removing the README.tools is better. Anyway, it's in the revisions history and previous releases. I've only left comment about <C>default</C> - I suggest to rephrase that, otherwise it's confusing.

@olexandr-konovalov
Copy link
Member

What about

If such a line is missing then it is assumed that the <F>manual.six</F> file
complies with the <F>gapmacro.tex</F> documentation, and reading this file is delegated to 
<C>HELP_BOOK_HANDLER.default.ReadSix</C>.

All this script does is to extract a tar file. Anybody who really
needs to do that can just do it directly, without need for a script.
@fingolfin
Copy link
Member Author

I rephrased in a somewhat different way, please take a look.

other that what you yourself impose. If such a line is missing then it is
assumed that the <F>manual.six</F> file complies with the <F>gapmacro.tex</F>
documentation format, which internally is referred to as the <C>default</C>
format for historical reasons. In that case reading the file is delegated to
Copy link
Member

Choose a reason for hiding this comment

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

perhaps I would even say "for legacy reasons", but that's not essential

Copy link
Member Author

Choose a reason for hiding this comment

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

Feel free to open a new PR which makes that change :-)

@olexandr-konovalov
Copy link
Member

@fingolfin I am happy for this to be now merged, thanks!

@fingolfin fingolfin merged commit 74a1680 into gap-system:master Jun 3, 2017
@fingolfin fingolfin deleted the mh/undoc-install-tools.sh branch June 6, 2017 15:02
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.0 milestone Jan 20, 2018
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 20, 2018
@nathancarter
Copy link

I'm not sure that these things were removed from all documentation, but I'm new to this, so forgive me if I'm getting this wrong. The current version of the GAP manual online still contains reference to tools.tar.gz and packpack, which don't exist in my latest GAP installation's etc/ folder. I was a bit confused and started googling around and ended up here.

@olexandr-konovalov
Copy link
Member

olexandr-konovalov commented Sep 26, 2018

Thanks @nathancarter. As the link https://www.gap-system.org/Manuals/pkg/example/doc/chapA.html shows, this is actually not the main GAP manual, but the Example package. Furthermore, https://www.gap-system.org/Packages/example.html links to the manual of its current version https://www.gap-system.org/Manuals/pkg/Example-4.1.1/doc/chap0.html which does not have that appendix at all. Before I will think of any action, it would be good to know how did you discover that link.

@nathancarter
Copy link

That's good to know. The old version is currently the top hit for the Google search "how to write a gap package."

@olexandr-konovalov
Copy link
Member

That's why I am always emphasising while teaching GAP that one should use GAP built-in tools to search in the documentation of GAP and all packages included in YOUR own installation instead of using Google: http://alex-konovalov.github.io/gap-lesson/01-command-line/.

For example, entering ??package would return

gap> ??package
Help: several entries match this topic - type ?2 to get match [2]

[1] Reference: package
[2] GAPDoc: Package
[3] Reference: Namespaces for GAP packages
[4] Reference: Using and Developing GAP Packages
[5] Reference: Installing a GAP Package
[6] Reference: Loading a GAP Package
[7] Reference: Automatic loading of GAP packages
[8] Reference: Functions for GAP Packages
[9] Reference: Kernel modules in GAP packages
[10] Reference: The PackageInfo.g File
[11] Reference: Guidelines for Writing a GAP Package
[12] Reference: Structure of a GAP Package
[13] Reference: An Example of a GAP Package
[14] Reference: Creating the PackageInfo.g File
...

@fingolfin
Copy link
Member Author

Still, we have outdated content on our website. IMHO, instead of trying to "educate" (which really isn't the right word here) people to not google (which obviously is hopeless, and in fact IMHO not sensible to start with), we should get our ship clean and remove or at least annotate outdated content.

@olexandr-konovalov
Copy link
Member

Would be nice to have something readthedocs-like... when multiple versions of the documentation could co-exist. Google would likely still be able to return older versions, though... /shrug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants