Improve the comments around the build target state#3217
Improve the comments around the build target state#3217Tatskaari wants to merge 1 commit intothought-machine:masterfrom
Conversation
| Active // Target is going to be used in current build | ||
| Pending // Target is ready to be built but not yet started. | ||
| Active // Target is going to be used in current build, but we're waiting for its dependencies to build | ||
| Pending // Target has been added to the build queue built but not yet started. |
There was a problem hiding this comment.
| Pending // Target has been added to the build queue built but not yet started. | |
| Pending // Target has been added to the build queue but not yet started. |
| Reused // Outputs of previous build have been reused. | ||
| Stopped // We stopped building the target because we'd gone as far as needed i.e. because we're only preparing the build dir for --shell | ||
| Built // Target has been successfully built. The target wasn't cached in any way. | ||
| Cached // Target has been retrieved from the build cache, but was not found in plz-out |
There was a problem hiding this comment.
but was not found in plz-out
It's unclear why this is relevant? Is this in comparison to Reused, where it was found in plz-out?
| Cached // Target has been retrieved from the cache | ||
| Unchanged // Target has been built but hasn't changed since last build | ||
| Reused // Outputs of previous build have been reused. | ||
| Stopped // We stopped building the target because we'd gone as far as needed i.e. because we're only preparing the build dir for --shell |
There was a problem hiding this comment.
Nit: is "we're only preparing the build dir for --shell" actually the only case where we'd see this statement?
If so, I suggest "We stopped building the target because we're only preparing the build dir for --shell"
If not, "i.e." should be "e.g."
| // target lifecycle. This property is used to make sure we never transition a target backwards i.e. you can't go back | ||
| // to Active from Built. | ||
| const ( | ||
| Inactive BuildTargetState = iota // Target isn't used in current build |
There was a problem hiding this comment.
Nit: please add trailing full stops consistently
| Cached // Target has been retrieved from the build cache, but was not found in plz-out | ||
| Unchanged // Target has been re-built (e.g. because the input or rule hash changed) but the outputs didn't change since last build | ||
| Reused // Outputs of previous build have been reused i.e. the outputs in plz-out were up-to-date | ||
| BuiltRemotely // Target has been built but outputs are not necessarily local. |
There was a problem hiding this comment.
What does "not necessarily local" mean? Not necessarily available in plz-out?
| Stopped // We stopped building the target because we'd gone as far as needed i.e. because we're only preparing the build dir for --shell | ||
| Built // Target has been successfully built. The target wasn't cached in any way. | ||
| Cached // Target has been retrieved from the build cache, but was not found in plz-out | ||
| Unchanged // Target has been re-built (e.g. because the input or rule hash changed) but the outputs didn't change since last build |
There was a problem hiding this comment.
| Unchanged // Target has been re-built (e.g. because the input or rule hash changed) but the outputs didn't change since last build | |
| Unchanged // Target has been re-built (e.g. because the input or rule hash changed) but the outputs didn't change since the last build |
|
This issue has been automatically marked as stale because it has not had any recent activity in the past 90 days. It will be closed if no further activity occurs. If you require additional support, please reply to this message. Thank you for your contributions. |
|
Do we want to try to merge this? @Tatskaari |
3 participants
Sat down with @goddenrich to clarify what each of these mean. Thought I'd update the comments to make it easier to understand the life cycle of a target from this enum.