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

large batch of changes #122

Merged
merged 9 commits into from
Apr 4, 2024
Merged

large batch of changes #122

merged 9 commits into from
Apr 4, 2024

Conversation

amonks
Copy link
Owner

@amonks amonks commented Apr 4, 2024

No description provided.

amonks added 9 commits April 4, 2024 10:50
Instead of reserving task IDs for internal use (which means that adding
any new internal task ID is a breaking change), we create a new
namespace with the '@' prefix for internal tasks.

BREAKING CHANGE: '@' is now a reserved character in task IDs.
closes #54

Before this change, we only used this stream for fsevents and the status
at the end of a run. The status events weren't especially useful: it's
weird to keep the UI open after a run is over, and we print exit status
after closing the UI anyway.

This change makes the stream a more purposeful:
- renames it to "watch" (I considered "files", but I like that "watch"
  matches the word used in taskfiles)
- removes the useless status events
- only shows the steram if the run includes file watchers
I do not understand what caused this issue, but somehow f48d166
introduced an error where tasks would report,

    error: wait: no child processes

I can't reproduce the error before that commit, and `run prerelease` (or
`run build`) consistently produces the error after it. This change
catches and ignores the error.

I feel very queasy about this change.
color.Render has a crash, but since it's only exercised by a `main`
package, it wasn't caught in tests. This change exercises the `main`
package a bit. It would be nice to do the integration tests this way,
too, but that's a project for another day.

This commit is a failing test; the next commit fixes it.
Fixes a crash that only showed up in run -list
f48d166 didn't actually create a stable ordering, because load returned
a map.
@amonks amonks merged commit ae20e30 into main Apr 4, 2024
1 check passed
@amonks amonks deleted the amonks/watch-stream branch April 4, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant