Add support for optional paths #472
Conversation
Quenty
force-pushed
the
users/quenty/optional
branch
from
c2f87b0
to
991b7a7
Compare
|
Note: This PR is designed to be merged as a semi-linear merge, or more likely, a regular merge. This will preserve commit history. |
|
I would not worry about preserving commit history here. Reviewing and merging PRs commit-by-commit is usually only helpful for incremental refactors, large changes, or a mix of mechanical and manual changes. I don't think this PR qualifies, and I have been reviewing it as a single chunk. |
…e defined either as `"$path": "src"` or `"$path": { "optional": "src" }`
Quenty
force-pushed
the
users/quenty/optional
branch
from
991b7a7
to
3ae5206
Compare
Quenty
force-pushed
the
users/quenty/optional
branch
from
3ae5206
to
49acd39
Compare
There was a problem hiding this comment.
Digging the latest round of changes!
It looks like there's a test failure, make sure you commit your updated Insta snapshot tests.
This should be the last feedback from me. If everything works from here, then I think we're good to go. I'd appreciate it if you would also file a PR on https://github.com/rojo-rbx/rojo.space with documentation updates! 😊
rojo-test/serve-tests/add_optional_folder/src/node_modules.project.json
Outdated
Show resolved
Hide resolved
|
I'll be taking a look at this today, sorry for the delay. |
Quenty
force-pushed
the
users/quenty/optional
branch
from
49acd39
to
069e2c8
Compare
LPGhatguy
dismissed
their stale review
Iterating to merge this in for 7.0 stable
* Add PathNode with optional fields to project. This allows a path to be defined either as `"$path": "src"` or `"$path": { "optional": "src" }`
* Make $path truly optional
* Prevent rojo from erroring if no project node is resolved
* Use match instead of if-statement
* Add end-to-end tests (credit to MobiusCraftFlip for initial scenario)
* Pass option with ref inside instead of reference to option
* Empty commit to restart GitHub Actions
* Simplify build test
* Minimize serve test: it fails
* Simplify serve test even more
* Ignore failing serve test
Co-authored-by: Lucien Greathouse <me@lpghatguy.com>
Closes #383 by adding the fields
"path": "src"or"path": { "optional": "src" }which indicates that the path is optional. Also adds a unit test by @MobiusCraftFlip adjusted for this format.I recommend reviewing this PR commit-by-commit for clarity.
This pull request enables scenarios with
node_modulesand other tools that sometimes create folders and sometimes does not.