Skip to content
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

Warning when installing new-style experimental packages #18566

Closed
nathanncohen mannequin opened this issue Jun 1, 2015 · 17 comments
Closed

Warning when installing new-style experimental packages #18566

nathanncohen mannequin opened this issue Jun 1, 2015 · 17 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jun 1, 2015

With #18563, which changes the status of several optional new-style packages to 'experimental', we now need to display a warning when these scripts are installed.

Nathann

Depends on #18563

CC: @vbraun @jdemeyer

Component: packages: experimental

Author: Nathann Cohen

Branch/Commit: 0d344a3

Reviewer: Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/18566

@nathanncohen nathanncohen mannequin added this to the sage-6.8 milestone Jun 1, 2015
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 1, 2015

Branch: public/18566

@nathanncohen nathanncohen mannequin added the s: needs review label Jun 1, 2015
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 1, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

ef11087Change known-broken packages to "experimental"
37fd689Valgrind doesn't build on OS X 10.10
b4d152btrac #18566: Warning when installing new-style experimental packages

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 1, 2015

Commit: b4d152b

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2015

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2015

comment:3

I would change

if [ `cat "$PKG_SCRIPTS/type"` == "experimental" ]

to

if [ x`cat "$PKG_SCRIPTS/type"` = xexperimental ]

which has less chances of trouble (in case of a missing/empty type file for example).

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2015

comment:4

Instead of

echo # The following message appears twice in this file

couldn't you just make a function warn_experimental() which displays that message?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

Changed commit from b4d152b to 6cafb51

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

6cafb51trac #18566: Bugfix

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 2, 2015

comment:6

I would change

if [ `cat "$PKG_SCRIPTS/type"` == "experimental" ]

to

if [ x`cat "$PKG_SCRIPTS/type"` = xexperimental ]

Nice. I did not know that corner-case, not that workaround. Done.

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2015

comment:7

= instead of == in shell scripts (in POSIX, only = is supported, bash happens to support == also)

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 2, 2015

comment:8

Yo !

Instead of

echo # The following message appears twice in this file

couldn't you just make a function warn_experimental() which displays that message?

I first did, then decided against it for the following reason: there is one message for experimental packages (appears twice) and one message for archived packages (appears once). As for old-style packages the two messages follow each other, it seemed weird to have one called from a function and another one given explicitly. Plus It would have meant that the new functions would have to be defined in the middle of unrelated code, somewhere else.

So well, I found it cleaner this way.

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

40a7807trac #18566: Bugfix

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

Changed commit from 6cafb51 to 40a7807

@jdemeyer
Copy link

jdemeyer commented Jun 5, 2015

comment:11

Please rebase to sage-6.8.beta3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 6, 2015

Changed commit from 40a7807 to 0d344a3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 6, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

0d344a3trac #18566: Merged with 6.8.beta3

@vbraun
Copy link
Member

vbraun commented Jun 6, 2015

Changed branch from public/18566 to 0d344a3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants