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

Revert workspace setting modified in hkust-taco#174 #217

Open
wants to merge 1 commit into
base: mlscript
Choose a base branch
from

Conversation

fo5for
Copy link
Collaborator

@fo5for fo5for commented Mar 24, 2024

https://github.com/hkust-taco/mlscript/pull/174/files#r1290512647
Turns out files.autoSave is indeed enabled by others.

@LPTK
Copy link
Contributor

LPTK commented Mar 26, 2024

I don't know actually... A lot of our testing infrastructure is set up to run some tests whenever test files are saved. Often, this can take a non-trivial amount of time. Letting users set files.autoSave in the project is likely to have them shoot themselves in the foot.

Do you have a good use case/workflow justifying the use of files.autoSave?

@LPTK
Copy link
Contributor

LPTK commented Mar 27, 2024

By the way, the workflow I'm currently working on, which is a huge improvement over the current diff-test workflow, relies heavily on file saves as a signal to run or re-run tests. This will be unusable if the user has the misfortune of having files.autoSave in their setting when they try using it.

@fo5for
Copy link
Collaborator Author

fo5for commented Mar 27, 2024

Instead of relying on file saves as signals, why not send the signal directly? You can bind a shortcut to send a sequence to the terminal. This solves the problem of the sbt watcher running twice back-to-back. Idk if the watcher also watches the source files or just the test files, but if does, it does not make sense to rerun the tests between each file save when modifying multiple source files together. The reason I enable autosave is that it has been massively helpful in averting my misfortune of losing progress.

@LPTK
Copy link
Contributor

LPTK commented Mar 27, 2024

Instead of relying on file saves as signals, why not send the signal directly?

Because file saves are also the natural checkpoint at which point it makes sense to run the test. We don't only use it as a signal. If we used a separate signal, one would have to BOTH save the file AND send the signal. And I really don't want to enable auto-save for myself (which wouldn't really solve the problem anyway).

You can bind a shortcut to send a sequence to the terminal.

The current solution is low-tech and editor-agnostic. This proposed approach is complex and editor-specific.

This solves the problem of the sbt watcher running twice back-to-back.

It replaces that problem (which is independently solvable) by something worse.

Idk if the watcher also watches the source files or just the test files, but if does, it does not make sense to rerun the tests between each file save when modifying multiple source files together.

Why not?

In a typical workflow, you'll have type errors anyway so the watcher won't be running until the project compiles. When it compiles, I want to see it run on the tests.

The reason I enable autosave is that it has been massively helpful in averting my misfortune of losing progress.

How do you normally lose progress? Editors like VSCode and Sublime always save your buffer's contents so that it will reopen it in the exact same state even if you reboot or the computer crashes.

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