Opened 6 years ago
Closed 6 years ago
#21500 closed enhancement (fixed)
Add instructions for using ccache
Reported by: | paulmasson | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-7.4 |
Component: | documentation | Keywords: | |
Cc: | Merged in: | ||
Authors: | Paul Masson | Reviewers: | Jeroen Demeyer, Matthias Koeppe |
Report Upstream: | N/A | Work issues: | |
Branch: | 0161c5c (Commits, GitHub, GitLab) | Commit: | 0161c5c3742d52ff4e9af8c5092a16051f1e5557 |
Dependencies: | Stopgaps: |
Description
Change History (21)
comment:1 Changed 6 years ago by
- Component changed from PLEASE CHANGE to documentation
- Priority changed from major to minor
- Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 6 years ago by
- Branch set to u/paulmasson/add_instructions_for_using_ccache
comment:3 Changed 6 years ago by
- Commit set to e22bbb45383e761795b12a4c2239416afa37d2d8
- Status changed from new to needs_review
comment:4 Changed 6 years ago by
- Commit changed from e22bbb45383e761795b12a4c2239416afa37d2d8 to 1cb5405e38b595872977ef5e84c88daeb42b4039
comment:5 Changed 6 years ago by
- Commit changed from 1cb5405e38b595872977ef5e84c88daeb42b4039 to b1f502e639b1ac232d5fe04d75f4a0b4dd395f9a
comment:6 Changed 6 years ago by
- Status changed from needs_review to needs_work
Can we please take this opportunity to clarify better that people really should use make
instead of ./sage -b
unless they know what they are doing. For example, your added paragraph seems to imply that it's fine to run just ./sage -b
when switching branches. However, in many cases that will not work because of changes to packages.
The text should make it clear that you need to run make
to compile Sage (or make build
to skip the documentation). If you made changes only to Sage library files, you can use ./sage -b
. When switching branches, usually you change more than that.
comment:7 Changed 6 years ago by
To clarify a bit better what I mean:
- The stuff you added about
ccache
really belongs with the part aboutmake
and not withsage -b
.
- Moreover, I would suggest to mention
make
first (so people become less drilled to usesage -b
all the time) andsage -b
after that.
For me, 1. must be fixed (hence needs_work), while 2. is an optional improvement that you may want to do.
comment:8 Changed 6 years ago by
Should these instructions go in the developer's guide at all? It might make more sense to put them in the installation guide, with a pointer from the developer's guide ("if you switch branches a lot, you might consider installing ccache: see ..."). The installation guide already has a very brief mention of ccache, for what that's worth.
comment:9 follow-up: ↓ 14 Changed 6 years ago by
Jeroen, the documentation as written is very clear on when to run sage -b
and make
. Since I'd assume it is mostly read by developers just getting started, the current order of instructions seems preferable to me. How many developers start out altering packages before Sage Python code?
I wanted my warning to be close to the statement "This should be quite fast", since that's what triggered the whole discussion for me. I can see now that inserting the warning in the middle of the section is confusing, so I'll move it to the end. That way it will apply to all of the rebuilding cases described above it.
John, the installation guide doesn't make clear why one would want to install ccache. The only reason I did is because I regularly switch branches when working on tickets. That's a development issue, not an installation issue, so I think it belongs here.
comment:10 Changed 6 years ago by
- Commit changed from b1f502e639b1ac232d5fe04d75f4a0b4dd395f9a to 82e686ef9cf7867089bf144d0fe50456f0f7f897
Branch pushed to git repo; I updated commit sha1. New commits:
82e686e | Add instructions for ccache
|
comment:11 Changed 6 years ago by
- Commit changed from 82e686ef9cf7867089bf144d0fe50456f0f7f897 to afdd55944a072a18ad99610d9d8021014ee7c12b
Branch pushed to git repo; I updated commit sha1. New commits:
afdd559 | Minor cleanup
|
comment:12 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:13 Changed 6 years ago by
typo Recynthonization -> Recythonization
comment:14 in reply to: ↑ 9 Changed 6 years ago by
Replying to paulmasson:
How many developers start out altering packages before Sage Python code?
Almost everybody does! But, just like you, they do not realize that changing branches often means changing packages. That is exactly the problem with the current documentation (although I agree that this is orthogonal to this ticket).
comment:15 Changed 6 years ago by
Nitpick: I do not think that WARNING
is correct here. Can you make it a NOTE
instead?
comment:16 Changed 6 years ago by
- Reviewers set to Jeroen Demeyer
- Status changed from needs_review to needs_work
Apart from this last nitpick, this is good for me.
comment:17 Changed 6 years ago by
- Commit changed from afdd55944a072a18ad99610d9d8021014ee7c12b to 0161c5c3742d52ff4e9af8c5092a16051f1e5557
Branch pushed to git repo; I updated commit sha1. New commits:
0161c5c | Corrections
|
comment:19 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:20 Changed 6 years ago by
- Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Matthias Koeppe
comment:21 Changed 6 years ago by
- Branch changed from u/paulmasson/add_instructions_for_using_ccache to 0161c5c3742d52ff4e9af8c5092a16051f1e5557
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Add instruction for ccache