Use BindGlobal instead of MakeReadOnlyGlobal

[fingolfin]

Note that to safely re-read a file which uses BindGlobal, one may use Reread or RereadPackage.

[Stefan-Kohl]

Using BindGlobal instead of MakeReadOnlyGlobal is probably a good idea -- but then the initializations of the lists of trial divisors in general.gi need to be removed. Also some care needs to be taken since the variables are then unbound before the corresponding files are read (check for IsBound instead of IsEmpty etc.).

[fingolfin]

Not sure what you mean with "the variables are then unbound before the corresponding files are read". Perhaps give an example?

[Stefan-Kohl]

Things are simpler to understand from the code than to explain in this case, but anyway: Look at https://travis-ci.org/gap-packages/FactInt/jobs/330027883 . The failure is due to attempts to use BindGlobal on variables which are already bound. So these variables shouldn't be initialised before BindGlobal is used to assign a value to them. But when they are not initialised, they are unbound before the corresponding files are read. E.g. before tables/factorial.g is read, K_FACTORIAL_M1_FACTORS is unbound then, and you cannot check for IsEmpty, but need to check for IsBoundGlobal instead.

[codecov[bot]]

Codecov Report

Merging #9 into master will increase coverage by 0.58%. The diff coverage is 68.57%.

@@            Coverage Diff             @@
##           master       #9      +/-   ##
==========================================
+ Coverage   84.94%   85.52%   +0.58%     
==========================================
  Files           6        6              
  Lines        1939     1914      -25     
==========================================
- Hits         1647     1637      -10     
+ Misses        292      277      -15

Impacted Files	Coverage Δ
lib/cfrac.gi	98.23% <100%> (ø)	:arrow_up:
lib/pplus1.gi	96% <100%> (ø)	:arrow_up:
lib/pminus1.gi	90.72% <100%> (ø)	:arrow_up:
lib/mpqs.gi	91.33% <100%> (ø)	:arrow_up:
lib/ecm.gi	92.57% <100%> (ø)	:arrow_up:
lib/general.gi	75.58% <59.25%> (+1%)	:arrow_up:

[fingolfin]

Ahh, I didn't notice the test failure, my bad. OK, the cleanest solution seems to simply always load those tables; they are tiny by modern standards, no need to delay loading them, or do any other complicated things (such as using InstallGlobalValue, or DeclareAutoreadableVariables)