-
-
Notifications
You must be signed in to change notification settings - Fork 529
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
MR38: Fix documentation of spkg-configure.m4 and make output more verbose #28788
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
comment:3
New commits added to merge request. I updated the commit SHA-1. New commits:
|
This comment has been minimized.
This comment has been minimized.
comment:5
New commits added to merge request. I updated the commit SHA-1. New commits:
|
comment:6
New commits added to merge request. I updated the commit SHA-1. New commits:
|
This comment has been minimized.
This comment has been minimized.
comment:8
I made some remarks on GitLab. Looking forward to comments at https://gitlab.com/sagemath/sage/merge_requests/38. |
comment:9
Also it would help to have a divider between the last test ("no suitable system package found for SPKG rw") and the start of the summary. I would prefer to see something like
|
comment:10
Ticket retargeted after milestone closed |
comment:12
Related: #28095 - Add |
This comment has been minimized.
This comment has been minimized.
comment:15
New commits added to merge request. I updated the commit SHA-1. New commits:
|
Reviewer: Matthias Koeppe |
comment:18
I get a merge failure with 9.1.beta3. Once fixed, please feel free to set back to positive review. |
comment:19
Actually, I have another request. The output in the ticket description, including
is what is printed on the screen, but the
with no text separating the last test from the summary. Can you add some text there so I can easily search Indeed, perhaps all of the summary text ("checking whether SageMath should install SPKG tachyon", etc.) should be included in the log file, not just on the screen. |
comment:20
To explain a bit more about my last comment: I am much more likely to look through the log file than to see the mass of text whiz by on the screen, so (assuming I'm not the only one) the log file should have formatting at least as good as what's on the screen. |
comment:21
I agree, everything that is printed on the screen should also appear in the log file. |
comment:22
I won't continue on this one. If you are unhappy with autoconf defaults, feel free to work around them. |
comment:23
mkoeppe: I don't know autoconf syntax, but if you can do the last fixes, I can review. |
Changed branch from u/galois/mrs/38/verbose-configure to u/mkoeppe/mrs/38/verbose-configure |
Branch pushed to git repo; I updated commit sha1. New commits:
|
This comment has been minimized.
This comment has been minimized.
Changed author from Julian Rüth to Julian Rüth, Matthias Koeppe |
Dependencies: #29120 |
comment:28
Replying to @jhpalmieri:
Ready for review. |
Changed reviewer from Matthias Koeppe to Matthias Koeppe, John Palmieri |
comment:29
Looks good to me, thanks! And thanks very much to saraedum for leading the efforts on this. Sorry to be so picky. |
comment:31
Thank you! |
Changed branch from u/mkoeppe/mrs/38/verbose-configure to |
comment:33
Follow up: #29211 |
Changed commit from |
This now produces the following output:
(Earlier version: merge request at https://gitlab.com/sagemath/sage/-/merge_requests/38)
Depends on #29120
CC: @embray @jhpalmieri @mkoeppe
Component: build: configure
Author: Julian Rüth, Matthias Koeppe
Branch:
bef6d57
Reviewer: Matthias Koeppe, John Palmieri
Issue created by migration from https://trac.sagemath.org/ticket/28788
The text was updated successfully, but these errors were encountered: