[Stacked PR] Remove warn_later from output #760
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
schneems
force-pushed
the
schneems/clean-background-timer-take-one
branch
from
87f4d6c to
47ac797
Compare
schneems
force-pushed
the
schneems/stacked-remove-warn-later
branch
from
6521058 to
a7406e7
Compare
The background timer prints dots to the UI while the buildpack performs some expensive operation. This requires we use threading and Rust really wants to be sure that we're not going to have data races and that our data is secure. The original version of the code relied on wrapping our IO in an Arc<Mutex<W>> so that we don't "lose" it when we pass it to the background thread. This worked, but was overly complicated based on my limited understanding of working with lifetimes and threads. This version instead relies on the ability to retrieve a struct when we join the thread to get the IO object back out. This reduces complexity that the BuildLog interface needs to know about. # This is the commit message #2: Inject tick state The tick values for the printer was hardcoded. Passing in the values allows us to remove the `style` module dependency and makes the tests simpler. # This is the commit message #3: Rename modules and add docs The prior name `print_dots` isn't technically valid anymore as it could be printing something else. I also moved the guard struct into it's own module to make it a bit safer (ensure that Option is always Some at creation time). # This is the commit message #4: Clarify intention that option is Some in code The debug_assert adds a stronger signal that the option must be Some when it is being constructed.
The `warn_later` functionality relies on global state in a way that we're not thrilled about putting into everyone's build packs by default. This removes the feature, plain and simple.
schneems
force-pushed
the
schneems/clean-background-timer-take-one
branch
from
47ac797 to
3a92e33
Compare
schneems
force-pushed
the
schneems/stacked-remove-warn-later
branch
from
a7406e7 to
141f892
Compare
Malax
approved these changes
Jan 19, 2024
schneems
changed the base branch from
schneems/clean-background-timer-take-one
to
schneems/output
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
warn_laterfunctionality relies on global state in a way that we're not thrilled about putting into everyone's build packs by default. This removes the feature, plain and simple.