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

Support for sprites on serve #143

Merged
merged 2 commits into from
Dec 20, 2022

Conversation

keichan34
Copy link
Contributor

Closes #137, closes #62

When writing sprite support, I saw that there were no tests for the charites serve command, so I added some simple tests. While there were initially tests in #80, they were removed before merging because of some timeout issue. I think I've resolved those issues in this PR.

Description

Adds sprite support to the serve command. Uses the same --sprite-input command line option as build.

Type of Pull Request

  • Adding a feature
  • Fixing a bug
  • Maintaining documents
  • Others ()

Verify the followings

  • Code is up-to-date with the main branch
  • No build errors after npm run build
  • No lint errors after npm run lint
  • No errors on using charites help globally
  • Make sure all the existing features working well
  • Have you added at least one unit test if you are going to add new feature?
  • Have you updated documentation?

Also creates a simple test for charites serve
Copy link
Contributor

@smellman smellman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@naogify naogify left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keichan34
Thank you! I find this feature very useful for map styling.

Sorry for the detail, but even though I have an icons directory, when I run charites serve style.yml, the icons do not show up on the map.

If user has an icons directory, I think charites should show local icons without explicitly specifying --sprite-input.

@keichan34
Copy link
Contributor Author

keichan34 commented Dec 20, 2022

@naogify

If user has an icons directory, I think charites should show local icons without explicitly specifying --sprite-input.

I agree. But, I don't think it's in the scope of this PR. I think that charites serve and charites build should have the same interface for building sprites -- this is a big part of why issues like #115 and #47 exist. I've opened an issue (#146) about unifying the interface and maybe specifying a special key in style.yml that indicates that sprites should be built, so sprites could be built automatically without specifying --sprite-input both on serve and build.

@naogify
Copy link
Contributor

naogify commented Dec 20, 2022

@keichan34
I understand it. I think #146 is nice idea! We have so much command line options right now. Thank you!

Copy link
Contributor

@naogify naogify left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@keichan34 keichan34 merged commit e4af1d1 into unvt:main Dec 20, 2022
@keichan34 keichan34 deleted the 137-sprites-serve-command branch December 20, 2022 04:06
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.

Support for sprites in the serve command Add unit test for serve command
3 participants