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

Add jsonOnly option to merge command #140

Merged
merged 7 commits into from
Jul 5, 2024
Merged

Conversation

jo-chemla
Copy link
Contributor

Following discussion thread #44

This option is meant to avoid copying resources (tileset.json and tile contents) when merging multiple input tilesets. Results in a tileset.json that references input tilesets where they are stored.

Can add another commit to remove the --jsonOnly flag from the merge command to instead create a new command, mergeJson

This option is meant to avoid copying resources (tileset.json and tile contents) when merging multiple input tilesets. Results in a tileset.json that references input tilesets where they are stored.
@javagl
Copy link
Contributor

javagl commented Jul 5, 2024

Thanks. I tried it out locally, and ran the tests, and verified that the CLA was signed, and everything seems to be fine....

...except for that line endings thing. Quickly looking at the documentation, it seems like --end-of-line lf is the default, so that should not be given explicitly. If this is removed, I can merge this PR.

There's still the question about the line endings, though: I'll have to check what the best practices/policies are here. This should be unrelated to this PR, and may be handled in a small cleanup PR later.

@jo-chemla
Copy link
Contributor Author

jo-chemla commented Jul 5, 2024

Woops sorry for pushing that change in package.json. Indeed this was just testing on my side, I reverted to the original package.json prettier command.

Edit: Note I also tried enforcing line endings to LF in .gitattributes at the root directry of the repo, it might be a good option to have it in the repo for future external windows-based contributors (eol can also be set to auto).

# .gitattributes
# Auto detect text files and perform LF normalization
* text eol=lf

@javagl
Copy link
Contributor

javagl commented Jul 5, 2024

Yes, it seems like using a .gitattributes is the way to go here. The desired behavior can also be configured by each user locally, but for the case that a user did not do this, the .gitattributes should take care of it. I'll confirm this internally, and likely create a PR to add this soon.

Thanks again for the mergeJson functionality.

(I'm usually updating things like the CHANGES.md and README.md prior to a release. So I'll probably schedule a release soon, and then also update these files accordingly)

@javagl javagl merged commit 2a47318 into CesiumGS:main Jul 5, 2024
1 check passed
@jo-chemla
Copy link
Contributor Author

Thanks for helping me design that PR, and happy that I could help!

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