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

maint: 🔧 update example app when honeycomb-opentelemetry-web files change #26

Merged
merged 4 commits into from
Jan 23, 2024

Conversation

ahrbnsn
Copy link
Contributor

@ahrbnsn ahrbnsn commented Jan 22, 2024

Short description of the changes

Adds dev command to the example app. Builds the source js, starts the server, and watches honeycomb-opentelemtry-web files for any changes. When there's a change, re-builds the example app's js.

How to verify that this has the expected result

  • pull the branch down locally.
cd examples/hello-world
npm run dev
  • open up the example app in your browser of choice
  • make a change to any of the source files in the root directory
  • refresh the example app

should be running the new code

@ahrbnsn ahrbnsn requested a review from a team as a code owner January 22, 2024 15:27
@pkanal pkanal added type: maintenance The necessary chores to keep the dust off. no-changelog Omit this PR from changelog/release notes. labels Jan 22, 2024
Copy link
Contributor

@pkanal pkanal left a comment

Choose a reason for hiding this comment

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

I'm so pumped for this! Going to make dev a lot easier!

"start": "serve .",
"start": "npm run build && serve .",
"watch:build": "chokidar \"../../src/*.ts\" -c \"npm run build\"",
"dev": "npm run start & npm run watch:build",
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried this out locally and made a change in src/honeycomb-otel-sdk.ts and it didn't end up showing up in the app. I can see that it is detecting a change in that file but I think the change isn't showing up because we're not rebuilding at the root level as well as the app level? I don't see the changes rebuilt in the dist folder.

Copy link
Contributor

@JamieDanielson JamieDanielson left a comment

Choose a reason for hiding this comment

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

The updated command works! Couple suggestions but otherwise this is great thanks!

updated-src-in-example

@@ -8,11 +8,16 @@ You can run this example to see the SDK in action.

Paste your API key into `index.js`, where is says "your api key goes here".

`npm run build`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can keep this because it's still a viable option, just may be preferred to use the dev option in local dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i tweaked the npm run start command so it calls npm run build itself, so folks wouldn't have to run both commands before starting up the server

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm going to merge, but definitely feel free to ignore & use the current existing setup in your 🔥 local dev instructions in #28

examples/hello-world-web/README.md Outdated Show resolved Hide resolved
@ahrbnsn ahrbnsn changed the title feat: ✨ update example app when honeycomb-opentelemetry-web files change maint: 🔧 update example app when honeycomb-opentelemetry-web files change Jan 23, 2024
@ahrbnsn ahrbnsn merged commit e320f9b into main Jan 23, 2024
8 checks passed
@ahrbnsn ahrbnsn deleted the ahr-dev-mode branch January 23, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Omit this PR from changelog/release notes. type: maintenance The necessary chores to keep the dust off.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants