-
Notifications
You must be signed in to change notification settings - Fork 182
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
Protect against syncing and building the wrong project types #772
base: master
Are you sure you want to change the base?
Conversation
You know, that's on me I didn't think to test whether our own tests would break if we changed this, I will fix it soon:tm:. It is currently 80 degrees and my laptop is hot though, so it'll have to wait. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that tests were relying on this behavior is pretty funny! 😂
While technically a breaking change, I doubt anyone ever actually intended to do this, or got a working result out of it... so it's probably fine?
Regarding #691, I think leaving that protection in the plugin is still valuable both as a failsafe and as protection against serves from otherwise compatible <=7.3.0 servers. |
Co-authored-by: Kenneth Loeffler <kenloef@gmail.com>
We can revisit tests later, since I'm not sure on the names of some of these. I want to keep the diff as relevant as possible though, since this turned into a bigger patch than anticipated. |
..Apologies for whatever the hell notification that just sent you, Ken. Github's UI isn't intuitive for this. |
Closes #736.
Prevents projects with a data model as a root from being built as a model file and vice versa. Also prevents syncing model files altogether.
This should mitigate a few of the more common problems with building Rojo projects. I'm not sure whether I should revert #691 or not. The check it adds is made moot with this PR, but it could be a good failsafe. Open to suggestion.
This doesn't impact the VSCode extension at all because it determines the file type to build as using a similar heuristic as this PR and listens to errors during serving to display to the user.