-
Notifications
You must be signed in to change notification settings - Fork 8
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
Change package banner to use BannerFunction #58
Conversation
Banner before this patch: Loading modules [mpfr, mpfi, mpc, fplll] for float 0.9.9 ... Banner after this patch: ───────────────────────────────────────────────────────────── Loading float 0.9.9 (Floating-point numbers) by Laurent Bartholdi (http://www.uni-math.gwdg.de/laurent). Loaded modules: mpfr, mpfi, mpc, fplll Homepage: https://gap-packages.github.io/float/ Report issues at https://github.com/gap-packages/float/issues ─────────────────────────────────────────────────────────────
Codecov Report
@@ Coverage Diff @@
## master #58 +/- ##
=======================================
Coverage 45.92% 45.92%
=======================================
Files 12 12
Lines 2430 2430
=======================================
Hits 1116 1116
Misses 1314 1314 |
@laurentbartholdi any thoughts on this PR? |
Hi Max!
No special thoughts... But I definitely prefer short, 1-line banners. Is
there a policy against them?
Also, I guess that your change was required by the
PackageInfo(...).BannerString becoming read-only in the future, so my
"self-modifying code" is not OK anymore. Is that so?
…On Mon, Nov 22, 2021 at 2:37 PM Max Horn ***@***.***> wrote:
@laurentbartholdi <https://github.com/laurentbartholdi> any thoughts on
this PR?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#58 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARAQUBNP2DS4Z46BJUX3XLUNJBRFANCNFSM5GFHDPYA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Laurent Bartholdi laurent.bartholdi<at>gmail<dot>com
Fachrichtung Mathematik, Universität des Saarlandes
Postfach 151150, 66041 Saarbrücken, Germany
Tel. +49 681 3023227, Sekr. +49 681 3023430
|
I'll try to make it a bit shorter, using your changes. |
I just pushed 1.0.0, to celebrate! Tell me if it's not OK to use the short format, which I prefer. |
You are of course free to format your banner the way you prefer, just like any other package author. I noticed that you also changed the minimal GAP version from 4.9 to 4.11. Any particular reason for that? Anyway, if that's what you want, then you should also remove the CI tests using GAP 4.9 & 4.10 (your change would have caused them to fail anyway, if it wasn't a for a quirk/bug in how those tests are setup (see <gap-actions/setup-gap#24.). |
Banner before this patch:
Banner after this patch: