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

updating documentation for ticket #11443 #12153

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

devkthines
Copy link
Contributor

@devkthines devkthines commented Aug 26, 2024

Description

I updated the instructions and included build instructions in the quickstart, i included the build in the build the code section before start. It was crucial so the build is ran first.

The change solves several potential problems:

Prevents Server Crashes: By ensuring that the npm run build command is run, all necessary files are generated before starting the server, preventing crashes or errors related to missing files.

Ensures Correct Asset Processing: It guarantees that all assets, such as GLSL shaders, are properly processed and available in their required formats, avoiding runtime issues and improving the reliability of the development server.

Provides a Clear Development Workflow: Adding the build step to the "Quickstart" section creates a clear and repeatable development process. This helps new developers or contributors quickly get up to speed and reduces confusion about the necessary setup steps.

Improves Documentation: Including the build step in the documentation ensures that it aligns with the actual development workflow and best practices. This helps avoid discrepancies between the documented process and the required steps to successfully run the project.

Issue number and link

##11443

Testing plan

Author checklist

  • [ X] I have submitted a Contributor License Agreement
  • [x ] I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • [x ] I have updated the inline documentation, and included code examples where relevant
  • [ x] I have performed a self-review of my code

Copy link

Thank you for the pull request, @devkthines! Welcome to the Cesium community!

In order for us to review your PR, please complete the following steps:

Review Pull Request Guidelines to make sure your PR gets accepted quickly.

@ggetz ggetz mentioned this pull request Aug 27, 2024
6 tasks
@ggetz
Copy link
Contributor

ggetz commented Aug 27, 2024

Thanks @devkthines! These changes look good. Once you bring over changes from #12154, this should be good to merge.

so the two branches are merged at the same time
@devkthines
Copy link
Contributor Author

Just closed #12154 and added that to this pull request. ready to merge!

@ggetz
Copy link
Contributor

ggetz commented Aug 27, 2024

Thanks @devkthines! After running CI, the linting checked failed. It looks like there is some minor markdown cleanup needed:

Documentation/Contributors/BuildGuide/README.md:22 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]
Documentation/Contributors/BuildGuide/README.md:27 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]
Documentation/Contributors/BuildGuide/README.md:72 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]

@devkthines
Copy link
Contributor Author

Updated, looks like it passed all tests. Thank you!

@ggetz
Copy link
Contributor

ggetz commented Aug 29, 2024

Thanks @devkthines!

@ggetz ggetz merged commit 2568dfe into CesiumGS:main Aug 29, 2024
4 checks passed
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