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

chore: introduce flix.allJobsFinished #395

Merged
merged 9 commits into from
Apr 14, 2024

Conversation

sockmaster27
Copy link
Contributor

Builds on #394

@sockmaster27 sockmaster27 changed the title All finished chore: introduce flix.allJobsFinished Mar 25, 2024
@sockmaster27 sockmaster27 changed the title chore: introduce flix.allJobsFinished chore: introduce flix.allJobsFinished Mar 25, 2024
@sockmaster27
Copy link
Contributor Author

While this still requires waiting on the file system watcher, my hope is that it's an overall improvement since it reduces the uncertainty of how long it takes to compile.

@magnus-madsen
Copy link
Member

Needs rebase then I will take a look :)

@sockmaster27
Copy link
Contributor Author

🫤

@magnus-madsen
Copy link
Member

I think the adding/removing stuff is complex. Perhaps we should add tests for all providers before we consider how to deal with this?

@magnus-madsen
Copy link
Member

Its possible the whole queue system has to be changed. But I would prefer to take smaller steps.

@sockmaster27
Copy link
Contributor Author

I can sort of recreate this using WSL but even more extreme where the diagnostics never even show up.
Maybe there's just a bug?
Could it be that the ordering of the connection is implemented in a non-ordered way on Linux such that the clearing signal arrives after the updated diagnostics?
Maybe it has to do with some of those error messages it's spewing out?

@magnus-madsen
Copy link
Member

I can sort of recreate this using WSL but even more extreme where the diagnostics never even show up. Maybe there's just a bug? Could it be that the ordering of the connection is implemented in a non-ordered way on Linux such that the clearing signal arrives after the updated diagnostics? Maybe it has to do with some of those error messages it's spewing out?

Unfortunately I have no idea. Perhaps you can explore what other extensions do?

@sockmaster27
Copy link
Contributor Author

@magnus-madsen
Possible workaround: Run tests on Windows or macOS instead.
We're not paying anything for the Actions on this repo are we? If we aren't, then I don't believe this would cost any thing extra.

@magnus-madsen
Copy link
Member

@magnus-madsen Possible workaround: Run tests on Windows or macOS instead. We're not paying anything for the Actions on this repo are we? If we aren't, then I don't believe this would cost any thing extra.

We are not paying anything no. But I don't know if we have access to Windows or Mac. Do you get that for free?

@sockmaster27
Copy link
Contributor Author

@magnus-madsen Possible workaround: Run tests on Windows or macOS instead. We're not paying anything for the Actions on this repo are we? If we aren't, then I don't believe this would cost any thing extra.

We are not paying anything no. But I don't know if we have access to Windows or Mac. Do you get that for free?

That's how I understand it: https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions
They have minute multipliers but if we have unlimited minutes I don't see how it would change anything.

@sockmaster27
Copy link
Contributor Author

🫤

@sockmaster27
Copy link
Contributor Author

@magnus-madsen
Could you try to retrigger the tests a couple of times to check if they're reliable?

@magnus-madsen
Copy link
Member

I have changed my mind and I now agree that adding some infrastructure to inspect the queue is the way to go.

@magnus-madsen magnus-madsen merged commit 3f06bda into flix:master Apr 14, 2024
7 checks passed
@sockmaster27 sockmaster27 deleted the all-finished branch April 14, 2024 08:33
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.

2 participants