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: add dev notes and update smoke test #28

Merged
merged 3 commits into from
Jan 23, 2024

Conversation

JamieDanielson
Copy link
Contributor

@JamieDanielson JamieDanielson commented Jan 22, 2024

Which problem is this PR solving?

  • my desire to add some notes after my first pass through as someone new to the project

Short description of the changes

  • Add dedicated DEVELOPING.md for local dev setup and notes, and reference that section in the README. Also update links in README for other docs.
  • Sometimes the console output takes longer than the default timeout of 4000ms, so update Cypress config to timeout at 10000ms instead.

How to verify that this has the expected result

Following any of the steps in the DEVELOPING doc work, including the smoke tests.

* Add dedicated DEVELOPING.md for local dev
setup and notes, and reference that section in
the README. Also update links in README.
* Console output is needed for smoke test, but
DiagConsoleLogger doesn't seem to be logging as
expected, maybe because of the bundler? So for
smoke tests right now, use ConsoleSpanExporter.
* Sometimes the console output takes longer than
the default timeout of 4000ms, so update Cypress
config to timeout at 10000ms instead.
@JamieDanielson JamieDanielson requested a review from a team as a code owner January 22, 2024 22:51
@JamieDanielson JamieDanielson self-assigned this Jan 22, 2024
@JamieDanielson JamieDanielson added type: maintenance The necessary chores to keep the dust off. version: no bump A PR with maintenance or doc changes that aren't included in a release. labels Jan 22, 2024
DEVELOPING.md Outdated
# start example app
npm start

# after PR 26
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If #26 merges first I can update this, otherwise I can come back to this... but wanted to note it while I'm here!

@@ -1,6 +1,7 @@
import { defineConfig } from 'cypress';

export default defineConfig({
defaultCommandTimeout: 10000, // default is 4000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When running the smoke tests locally the tests would fail with a timeout, so I picked an arbitrary number for now and it's been more reliable

Copy link

@mjayaram mjayaram left a comment

Choose a reason for hiding this comment

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

All instructions so far worked for me locally! Thanks

Copy link
Contributor

@ahrbnsn ahrbnsn left a comment

Choose a reason for hiding this comment

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

glorious! 🚀

just needed to enable Verbose level
in the console. Removing the
console span exporter
@JamieDanielson JamieDanielson merged commit 64e0888 into main Jan 23, 2024
7 checks passed
@JamieDanielson JamieDanielson deleted the jamie.dev-and-local-setup branch January 23, 2024 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: maintenance The necessary chores to keep the dust off. version: no bump A PR with maintenance or doc changes that aren't included in a release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants