-
Notifications
You must be signed in to change notification settings - Fork 161
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
ENHANCE: CharacteristicSubgroupsLib to CharacteristicSubgroups #978
Conversation
Current coverage is 49.61% (diff: 95.65%)@@ master #978 diff @@
==========================================
Files 424 424
Lines 222957 222959 +2
Methods 3430 3430
Messages 0 0
Branches 0 0
==========================================
- Hits 110661 110629 -32
- Misses 112296 112330 +34
Partials 0 0
|
I've also copied "Also remove multi-declaration warning if both declarations are equal -- this allows the library to duplicate declarations from packages" from the commit message to the comment above - IMHO its important change so it should be more visible there. |
This looks sensible. However, these are really two different changes which are bunched together, and one affects all of GAP by modifying how declarations work -- so I'd consider this the by far bigger change. Yet it is not mentioned in the headline of the commit message. Therefore I'd greatly prefer if the commits could be split into two (I think this it is in general a good idea to keep independent changes in separate commits). This can be done in under two minutes like this: git checkout branch-with-this-work # here I assume you have a clean work tree...
git reset HEAD^ # undo the last commit, but leave the changes in work tree
git add -p # use interactive add to only stage the changes for first commit
git commit # make first commit
git add -p # stage remaining changes
git commit Of course, since the two changes split along file borders nicely, one can also simply explicitly "git add FILES" instead of an interactive add. |
@fingolfin |
c95f52e
to
334d22a
Compare
This allows the library to duplicate declarations from packages.
@hulpke Yes, exactly as you have done it. Thank you very much! |
This PR renames CharacteristicSubgroupsLib to CharacteristicSubgroups (as suggested in #975).
As previously discussed in #878, it avoids problems with the declaration in CRISP by removing the ``multiple declarations match'' error, if both declarations are the same.
The actual functionality provided (if CRISP is not loaded) is pathetic and not worth mentioning in the release announcement.
Copied by @alex-konovalov from commit message: "Also remove multi-declaration warning if both declarations are equal -- this allows the library to duplicate declarations from packages"