Added guidelines for package authors from Example package #484

olexandr-konovalov · 2016-01-17T21:33:42Z

I have started to work on moving the guidelines for package authors from the Appendix of the Example package to the chapter on GAP packages in the reference manual.

This is work in progress - at the moment I copied and pasted the text from Appendix with minimal edits.

Please help to review the text and identify things that have to be rewritten, leaving your comments under corresponding lines in the commit.

UPDATE: this finally acquired some shape and I am happy to start the review process. Comments are welcome. You may decide to comment on the cumulative diff or on the two commits: the 1st commit just copies the text from the Appendix of the Example package manual, and the second edits it, so you can see which parts of the text are inherited from the old document, and which have been changed.

olexandr-konovalov · 2016-01-17T22:08:39Z

Seems like because of the diff of 2000+ lines one can't comment on individual lines of the commit :(

olexandr-konovalov · 2016-01-17T22:25:57Z

This is a limitation of GitHub - it will not show diffs for commits larger than it seems 1500 lines.
I've now added the new material in two commits instead of one and force-pushed it. You will still not see the cumulative diff under "files changed" but you will be able to see the added text and leave your comments under "commits".

olexandr-konovalov · 2017-03-01T22:05:02Z

This something that I still want and intend to do. Assigned to myself and set milestone 4.9.0.
Cross-referenced to gap-packages/example#8

codecov · 2017-03-11T00:09:39Z

Codecov Report

Merging #484 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #484      +/-   ##
==========================================
+ Coverage   63.49%   63.49%   +<.01%     
==========================================
  Files         952      952              
  Lines      286804   286803       -1     
  Branches    12747    12747              
==========================================
+ Hits       182103   182104       +1     
+ Misses     101904   101897       -7     
- Partials     2797     2802       +5

Impacted Files	Coverage Δ
src/hpc/thread.c	`45.25% <0%> (-0.8%)`	⬇️
src/stats.c	`79.01% <0%> (-0.28%)`	⬇️
src/lists.c	`56.6% <0%> (-0.12%)`	⬇️
src/hpc/threadapi.c	`34.3% <0%> (ø)`	⬆️
src/funcs.c	`74.75% <0%> (+0.14%)`	⬆️
src/objset.c	`82.59% <0%> (+0.23%)`	⬆️
src/iostream.c	`46.64% <0%> (+0.39%)`	⬆️
src/hpc/traverse.c	`78.68% <0%> (+0.68%)`	⬆️
lib/queue.g	`69.59% <0%> (+3.19%)`	⬆️

olexandr-konovalov · 2017-03-11T22:55:53Z

I did some minor rewrites and added numerous TODOs to the package guidance - will try to get more progress this week, aiming at having this update for GAP 4.9 and making new release of the Example package with this appendix removed.

olexandr-konovalov · 2017-09-11T23:41:06Z

Update: trying to make some progress with this during this week.

olexandr-konovalov · 2017-09-17T17:38:42Z

This finally acquired some shape and I am happy to start the review process. Comments are welcome. You may decide to comment on the cumulative diff or on the two commits: the 1st commit just copies the text from the Appendix of the Example package manual, and the second edits it, so you can see which parts of the text are inherited from the old document, and which have been changed.

fingolfin · 2017-09-18T08:08:29Z

doc/ref/gappkg.xml

-or install new packages between upgrades of your &GAP; installation.
+Before a  package can be  used it must  be installed. A standard
+installation of &GAP; already contains all currently redistributed with 
+&GAP; packages. Such combination of packages is checked for compatibility


Word order: "all GAP packages currently distributed with GAP".

Then: "This set of packages is checked ..."

fingolfin · 2017-09-18T08:09:09Z

doc/ref/gappkg.xml

+the packages can be used immediately, but some of them may require further
+installation steps such as compilation or installing additional software
+to satisfy their dependencies. Most of the packages that require compilation
+will be compiled in a single step by changing to the <F>pkg</F> directory 


Will -> can

fingolfin · 2017-09-18T08:09:51Z

doc/ref/gappkg.xml

+installation steps such as compilation or installing additional software
+to satisfy their dependencies. Most of the packages that require compilation
+will be compiled in a single step by changing to the <F>pkg</F> directory 
+of your &GAP; installation and calling <C>../bin/BuildPackages.sh</C> script.


"... calling the ... script."

fingolfin · 2017-09-18T08:12:12Z

doc/ref/gappkg.xml

+of your &GAP; installation and calling <C>../bin/BuildPackages.sh</C> script.
+<P/>
+Also, since &GAP; packages are released independently of the main &GAP; system,
+sometimes it may be sensible to upgrade or install new packages between


Not sure "sensible" is a good word choice here. Perhaps "useful"? And then perhaps clarify why/ when, e.g. like this: "... e.g. if a new version of a package adds new capabilities or bug fixes that you need."

fingolfin · 2017-09-18T08:14:11Z

doc/ref/gappkg.xml

-<Ref Func="LoadPackage"/>.
+Some &GAP; packages are loaded automatically with &GAP;. Those belong to
+two categories: packages which are needed to start &GAP; (their list is 
+contained in <C>GAPInfo.Dependencies.NeededOtherPackages</C>), and packages


I'd make this explicit: "(as of this writing, the only such package is GAPDoc; the current list is contained in the ... variable)"

fingolfin · 2017-09-18T11:59:29Z

doc/ref/gappkg.xml

+in loading it via <Ref Func="LoadPackage"/>, satisfying 
+all specified dependencies.
+<P/>
+You can also try to create a basic &GAP; package with the help of the


Remove "try" ?

And in light of the above, perhaps insert before this a paragraph which mentions validation and/or refs to ValidatePackageInfo, and something along the lines of "While this minimal PackageInfo.g is enough to let GAP load the package, it actually is missing many extra fields that every such file should contain, and which we require for a package to be redistributed with GAP. The command ValidatePackageInfo can be used to get a quick idea about which fields are missing:

gap> ValidatePackageInfo("PackageInfo.g"); #E component `Subtitle' must be bound to a string #E component `Date' must be bound to a string of the form `dd/mm/yyyy' #E component `ArchiveURL' must be bound to a string started with http://, https:// or ftp:// #E component `ArchiveFormats' must be bound to a string #E component `Status' must be bound to one of "accepted", "deposited", "dev", "other" #E component `README_URL' must be bound to a string started with http://, https:// or ftp:// #E component `PackageInfoURL' must be bound to a string started with http://, https:// or ftp:// #E component `AbstractHTML' must be bound to a string #E component `PackageWWWHome' must be bound to a string started with http://, https:// or ftp:// #E component `ArchiveURLSubset' must be bound to a list of strings denoting relative paths to readable files or directories #E component `HTMLStart' must be bound to a string denoting a relative path to a readable file #E component `PDFFile' must be bound to a string denoting a relative path to a readable file #E component `SixFile' must be bound to a string denoting a relative path to a readable file #E component `LongTitle' must be bound to a string false

At that point, it then is quite natural to bring up PackageMaker, and to mention that it takes care of almost all of that for you.

Aha, and a link to the "Creating the PackageInfo.g File" section should be added here, too. And the log I gave above could be added there (instead), I guess.

fingolfin · 2017-09-18T12:00:18Z

doc/ref/gappkg.xml

+all specified dependencies.
+<P/>
+You can also try to create a basic &GAP; package with the help of the
+tool called <Package>PackageMaker</Package>, created by Max Horn and


It is my hope that others will help out with that tool, so perhaps don't bring up my name with that? The manual doesn't mention Max N. and Frank for GAPDoc all the time either, I think.

fingolfin · 2017-09-18T12:01:32Z

doc/ref/gappkg.xml

+<Log><![CDATA[
+#############################################################################
+##
+#F  ListDirectory([<dir>])  . . . . . . . . . . list the files in a directory


Personally, I would just ditch this line. It serves no purpose; but having it here makes people feel like they need to add it, increasing the work required to write comments. Also, these lines tend to get out of sync with the rest of the comment anyway."

fingolfin · 2017-09-18T12:02:35Z

doc/ref/gappkg.xml

+This description was automatically be extracted to build the
+manual source, if there is a <C>\Declaration</C> line in some
+<C>.msk</C> file together with an appropriate configuration file.
+<P/>


Personally, I'd remove the stuff between Historicallyand this point. Well, or perhaps instead move it to a new "historical" section (the information above is useful for understanding why some old code has comments like that). But other than that historical interest, I find it quite distracting. Prospective package authors have to slog through all of that for no good reason before they get to the stuff that actually matters for them.

fingolfin · 2017-09-18T12:07:54Z

doc/ref/gappkg.xml

+<Ref Func="ValidatePackageInfo"/>.
+
+</Section>
+


I stopped reading here for now. Perhaps somebody else can pick up reviewing after this, or perhaps I'll find some time and energy to do it later.

olexandr-konovalov · 2017-10-21T17:54:31Z

@fingolfin this has been updated following your comments on the initial part of the chapter.

olexandr-konovalov · 2017-11-03T00:06:28Z

I have taken all feedback received so far from @fingolfin into account. I would like to see this merged for 4.9 beta, and then update the Example package, to remove the old version of this text from there. We then may point package author to its new location in the Ref manual, and then hopefully get further feedback from them after publishing beta.

fingolfin · 2017-11-03T20:09:56Z

doc/ref/gappkg.xml

+and uses the value assigned to <C>GAParch</C> in the file <F>sysinfo.gap</F>,
+created when &GAP; was compiled to determine the
+compilation architecture, inserts this in place of the string <C>@GAPARCH@</C>
+in <F>Makefile.in</F> and creates a file <F>Makefile</F>. When <C>make</C> is


This information is outdated with the new build sys. While this still works (due to the "compatibility mode"), the plan is for sysinfo.gap to go away, or at last change substantially. I am not comfortable telling people to use it this way. :/

That said, right now the alternatives are not yet ready, so we may have no choice. I'll try to work on the package build system side next week, but unless I make progress, we'll just have to merge this, but I must make a big note somewhere to remind me that this piece of text exists and needs to be updated.

I've put the comment in the XML source about this.

fingolfin · 2017-11-03T20:10:48Z

doc/ref/gappkg.xml

+GAP4stones: 3333
+true
+]]></Log>
+Tests which produce extended output and/or requires substantial runtime


requires -> require

fingolfin · 2017-11-03T20:13:01Z

doc/ref/gappkg.xml

+works. Such tests should be included in the package distribution, and the
+responsibility for ensuring that they pass stays with package authors.
+<P/>
+Second, the package should cleanly intergate into the &GAP; system and other


intergate -> integrate

fingolfin · 2017-11-03T20:14:31Z

doc/ref/gappkg.xml

+documentation format that is defined in and can be used with the &GAPDoc;
+package (see&nbsp;<Ref Chap="Introduction and Example" BookName="gapdoc"/>).
+<P/>
+There should be at least a text version of your documenation provided for use


documenation -> documentation

fingolfin · 2017-11-03T20:14:58Z

doc/ref/gappkg.xml

+<P/>
+Additionally, there is a recommended naming convention that the &GAP; core
+system and &GAP; packages should not use global variables starting in the
+lowercase. This allows to reserve variables with names starting in lowecase


lowecase -> lowercase

fingolfin · 2017-11-03T20:16:02Z

doc/ref/gappkg.xml

+the file <F>doc/example.xml</F> of the <Package>Example</Package> package.
+(it uses <Package>GAPDoc</Package> for documentation -
+you will need some different scheme to achieve this using
+the <F>gapmacro.tex</F> format).


Remove the last part in parenthesis?

Checked and removed some other mentions of gapmacro. Now the only one left is "Some packages still use for their manuals the old gapmacro format, support for which may be discontinued in the future. We encourage authors of those packages to eventually convert their documentation &GAPDoc;."

fingolfin · 2017-11-03T20:16:35Z

doc/ref/gappkg.xml

+</Subsection>
+
+
+<Subsection Label


aplhabetical -> alphabetical

fingolfin · 2017-11-03T20:17:42Z

doc/ref/gappkg.xml

+</Subsection>
+
+
+<Subsection Label


permisisons -> permissions

fingolfin · 2017-11-03T20:18:07Z

doc/ref/gappkg.xml

+</Subsection>
+
+
+<Subsection Label


maybe replace that with .gitignore?

olexandr-konovalov · 2017-11-04T14:37:26Z

@fingolfin thanks. Addressed comments and run a spellcheck. Should be ready to merge if Travis tests pass.

cdwensley · 2017-11-05T14:26:16Z

doc/ref/gappkg.xml

 (<URL>https://www.gap-system.org</URL>)
 and in the &GAP; package <Package>Example</Package>
 (see <URL>https://www.gap-system.org/Packages/example.html</URL>).
 There  are  possibilities  to get  a  package  distributed together  with
 &GAP;  and it  is possible  to submit  a package  to a formal refereeing
 process.
 <P/>
-In this chapter we describe how to use existing packages.
+In this chapter first we describe how to use existing packages,


cdwensley · 2017-11-05T14:35:38Z

doc/ref/gappkg.xml

@@ -27,26 +17,36 @@ integrated into the &GAP; help system.
 All  &GAP;  users   who  develop  new  code  are   invited  to  share
 the  results  of their  efforts  with  other  &GAP; users  by  making
 the  code  and its  documentation  available  in  form of  a  package.
-Information how  to do  this is  available from  the &GAP;  Web pages
+Information how  to do  this is  available from  the &GAP; website
 (<URL>https://www.gap-system.org</URL>)
 and in the &GAP; package <Package>Example</Package>
 (see <URL>https://www.gap-system.org/Packages/example.html</URL>).
 There  are  possibilities  to get  a  package  distributed together  with


This sentence needs changing. Perhaps (working from the website): Once a package works and has documentation of its functionality, you should consider how to make it available to other GAP users as a deposited package. Please contact [email protected] so that the GAP Group can make the necessary checks. In these are satisfied, the package can be included in the distribution update mechanism.

I'd leave it as it is - there are more details below about depositing a package.

cdwensley · 2017-11-05T17:36:24Z

doc/ref/gappkg.xml

-that is they will be loaded automatically with &GAP;,
-others must in each case be separately loaded by a call to
-<Ref Func="LoadPackage"/>.
+Some &GAP; packages are loaded automatically with &GAP;. Those belong to


Surely the average (or casual) GAP user just wants to know how to load a package? So include here the material in the LoadPackage subsection which refers to this, and move the automatic loading stuff to a later subsection.

I've reordered the text to mention LoadPackage at the start of the Section.

pedritomelenas · 2017-11-06T22:25:22Z

doc/ref/gappkg.xml

+<Ref Sect="sect:gap.ini"/>) or start &GAP; with <C>-O</C> command line
+option (this may also cause problems with loading other packages using
+<Q>obsolete</Q> variables).
+


gap -O might be a bit discouraging; probably gap -A -O would be better, since even many default autoloaded packages contain obsolete functions

@pedritomelenas thanks, good suggestion!

fingolfin

It doesn't look like many people are interested in further checking this. Yet it represents a clear improvement over what is currently there. So, I am fine with merging it now and improving it later, in-tre.

olexandr-konovalov added topic: documentation Issues and PRs related to documentation gapsagedays2016 Issues and PRs that arose at https://www.gapdays.de/gap-sage-days2016 labels Jan 17, 2016

olexandr-konovalov force-pushed the package-manual branch from 60d1853 to f2adf92 Compare January 17, 2016 22:21

olexandr-konovalov mentioned this pull request Jan 20, 2016

harmonise names of package dirs in pkg/ gap-system/gap-distribution#8

Closed

olexandr-konovalov added the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Jan 22, 2016

olexandr-konovalov mentioned this pull request Feb 29, 2016

cleaning up package sources for distribution gap-system/gap-distribution#7

Closed

olexandr-konovalov mentioned this pull request Feb 20, 2017

Move GAPDocManualLab and GAPDocManualLabFromSixFile from GAP to GAPDoc, and possibly document them frankluebeck/GAPDoc#5

Closed

olexandr-konovalov mentioned this pull request Mar 1, 2017

Move guidelines to the GAP reference manual gap-packages/example#8

Closed

olexandr-konovalov self-assigned this Mar 1, 2017

olexandr-konovalov added this to the GAP 4.9.0 milestone Mar 1, 2017

olexandr-konovalov force-pushed the package-manual branch from 0c28623 to c3378a7 Compare March 10, 2017 23:27

olexandr-konovalov added the gapdays2017-spring Issues and PRs that arose at https://www.gapdays.de/gapdays2017-spring label Mar 12, 2017

olexandr-konovalov force-pushed the package-manual branch from 4fa60e8 to 082062e Compare September 11, 2017 17:10

olexandr-konovalov force-pushed the package-manual branch 4 times, most recently from 8f109fa to f11a1b9 Compare September 17, 2017 17:33

olexandr-konovalov removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Sep 17, 2017

olexandr-konovalov requested review from fingolfin, ChrisJefferson, markuspf, stevelinton, james-d-mitchell and hulpke September 17, 2017 17:34

olexandr-konovalov requested review from cdwensley and Stefan-Kohl September 17, 2017 17:35

olexandr-konovalov changed the title ~~Added guidelines for package authors from Example package (WIP)~~ Added guidelines for package authors from Example package Sep 17, 2017

olexandr-konovalov requested a review from frankluebeck September 17, 2017 17:51

fingolfin reviewed Sep 18, 2017

View reviewed changes

olexandr-konovalov force-pushed the package-manual branch from f11a1b9 to ee6ec52 Compare October 6, 2017 23:52

olexandr-konovalov force-pushed the package-manual branch 2 times, most recently from 9cf04fd to 6781b94 Compare October 21, 2017 17:53

fingolfin reviewed Nov 3, 2017

View reviewed changes

olexandr-konovalov force-pushed the package-manual branch from 6781b94 to 72fad4f Compare November 4, 2017 14:36

cdwensley reviewed Nov 5, 2017

View reviewed changes

olexandr-konovalov force-pushed the package-manual branch from 72fad4f to af06996 Compare November 6, 2017 21:32

pedritomelenas reviewed Nov 6, 2017

View reviewed changes

Alexander Konovalov added 2 commits November 6, 2017 22:42

Copied guidelines for package authors from Example package

361bf64

Rewrite guidelines for package authors

b9ba7e6

olexandr-konovalov force-pushed the package-manual branch from af06996 to b9ba7e6 Compare November 6, 2017 22:43

fingolfin approved these changes Nov 6, 2017

View reviewed changes

fingolfin merged commit c189950 into gap-system:master Nov 7, 2017

olexandr-konovalov mentioned this pull request Nov 7, 2017

Future plans for the Example package gap-packages/example#9

Open

cdwensley mentioned this pull request Nov 8, 2017

split manual 76.2.1 into two subsections #1877

Merged

olexandr-konovalov deleted the package-manual branch November 14, 2017 22:41

olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Dec 14, 2017

		</Subsection>


		<Subsection Label

		</Subsection>


		<Subsection Label

		</Subsection>


		<Subsection Label

Added guidelines for package authors from Example package #484

Added guidelines for package authors from Example package #484

Conversation

olexandr-konovalov commented Jan 17, 2016 • edited Loading

olexandr-konovalov commented Jan 17, 2016

olexandr-konovalov commented Jan 17, 2016

olexandr-konovalov commented Mar 1, 2017

codecov bot commented Mar 11, 2017 • edited Loading

Codecov Report

olexandr-konovalov commented Mar 11, 2017

olexandr-konovalov commented Sep 11, 2017

olexandr-konovalov commented Sep 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olexandr-konovalov commented Oct 21, 2017

olexandr-konovalov commented Nov 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olexandr-konovalov commented Nov 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fingolfin left a comment

Choose a reason for hiding this comment

olexandr-konovalov commented Jan 17, 2016 •

edited

Loading

codecov bot commented Mar 11, 2017 •

edited

Loading