-
Notifications
You must be signed in to change notification settings - Fork 22
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
Show banner again by default #429
Conversation
Codecov Report
@@ Coverage Diff @@
## master #429 +/- ##
==========================================
- Coverage 73.47% 73.43% -0.04%
==========================================
Files 63 63
Lines 5055 5060 +5
==========================================
+ Hits 3714 3716 +2
- Misses 1341 1344 +3
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, inline with the other corners
I also would prefer to see the banner. I am also thinking of a banner for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gap_module
is a local variable in __init__
,
perhaps use it as an argument of run_it
in order to use it inside this function.
My personal preference is not to show banners, but if there is an agreement that every cornerstone shall show a banner then I do not object.
(Wasn't there a convention for Julia packages to not show banners, see #334?)
@ThomasBreuer regarding Banners, see oscar-system/Oscar.jl#92 |
... unless we are being loaded from Oscar (code for that taken from Singular.jl); can (still) be overridden with the `GAP_SHOW_BANNER` environment variable.
@ThomasBreuer fixed the gap_module issue |
On Thu, Apr 30, 2020 at 07:50:43AM -0700, ThomasBreuer wrote:
@ThomasBreuer requested changes on this pull request.
`gap_module` is a local variable in `__init__`,
perhaps use it as an argument of `run_it` in order to use it inside this function.
My personal preference is not to show banners, but if there is an
agreement that every ornerstone shall show a banner then I do not
object. (Wasn't there a convention for Julia packages to not show
banners, see #334?)
Yes to both. We, as PIs decided that our end-products have banners -
when used interactively
And yes, julia decided against a banner - except for Julia.
…
--
You are receiving this because your review was requested.
Reply to this email directly or view it on GitHub:
#429 (review)
|
... unless we are being loaded from Oscar (code for that taken from
Singular.jl); can (still) be overridden with the
GAP_SHOW_BANNER
environment variable.
This is based on the recent PI decision on banners.
Note that this simply shows the default GAP banner, e.g.:
I could change it to a custom banner, of course (discussion on that is ongoing between the PIs)