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

chore: #148 upgrade examples #157

Merged
merged 7 commits into from
Apr 29, 2024
Merged

chore: #148 upgrade examples #157

merged 7 commits into from
Apr 29, 2024

Conversation

miquelbeltran
Copy link
Contributor

@miquelbeltran miquelbeltran commented Apr 24, 2024

chore: #148 upgrade examples

Description 📝

Take the two examples in the repo and upgrade dependencies. Also ensure that they use the raygun4node found in the repo, not the one from npm.

Type of change

  • Tech debt ;)

Updates

  • Upgrade dependencies in examples/express-sample.
  • Upgrade dependencies in examples/using-domains.
  • Ensure both examples use the raygun node package from the repo, not the one hosted in npm.
  • Adds multiple console.log to examples/using-domains.
  • Fixes indentation in examples/using-domains/app.js.

Note: The eslint config is done in #156 and includes the files in the examples folders.

Test plan 🧪

  • Run both examples.

Author to check 👓

  • Project and all contained modules builds successfully
  • Self-/dev-tested
  • Unit/UI/Automation/Integration tests provided where applicable
  • Code is written to standards
  • Appropriate documentation written (code comments, internal docs)

Reviewer to check ✔️

  • Project and all contained modules builds successfully
  • Change has been dev-/reviewer-tested, where possible
  • Unit/UI/Automation/Integration tests provided where applicable
  • Code is written to standards
  • Appropriate documentation written (code comments, internal docs)

@miquelbeltran miquelbeltran marked this pull request as ready for review April 24, 2024 10:04
Copy link
Contributor

@TheRealAgentK TheRealAgentK left a comment

Choose a reason for hiding this comment

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

LGTM overall, haven't built/tested it myself yet though. Will do later and then approve/merge the PR.

Copy link
Collaborator

@sumitramanga sumitramanga left a comment

Choose a reason for hiding this comment

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

Code also looks good to me! I'll look to spin this up for testing and follow the instructions

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be awesome to have the README more filled in on how to run the example apps!

I didn't realise we had to build the provider first before building and running the app! This tripped me up badly as I was wondering why TS was needed in the example app as the example app didn't have TS but the provider itself does. Went down a rabbit hold 😬 But realised the situation as mentioned before.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we just need to add in "ensure you've built the provider in the root directory before running this example app". Same goes for the other example app

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to explicitly point out the API Key change to ensure that the config value is changed to their API Key? Just to make it obvious to the user

Copy link
Collaborator

@sumitramanga sumitramanga left a comment

Choose a reason for hiding this comment

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

Just one small change that people would benefit having IMO :)

Otherwise, testing went well and the apps are working with error being piped into Raygun 🙌

Copy link
Collaborator

@sumitramanga sumitramanga left a comment

Choose a reason for hiding this comment

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

Oh, one other issue I did have with the express example app was that I had to run set DEBUG=express-sample:server && node ./bin/www instead of npm start to get the server up and running. Not sure why, but the server just terminates when running npm start so the other option that worked was the manual run in terminal.

Let me know how you guys get on with npm start @miquelbeltran @TheRealAgentK ? It could just be a local issue on my end perhaps...

@miquelbeltran
Copy link
Contributor Author

miquelbeltran commented Apr 26, 2024

I didn't realise we had to build the provider first before building and running the app!

I've added that step now to the README

Do we need to explicitly point out the API Key change to ensure that the config value is changed to their API Key? Just to make it obvious to the user

I added now a check in both examples.

Not sure why, but the server just terminates when running npm start so the other option that worked was the manual run in terminal.

I did not have this issue, that's strange...

@sumitramanga
Copy link
Collaborator

Thank you for addressing those @miquelbeltran !
As for the local set up issues, I don't think this is tied to provider issues themselves but a problem on my computer. So no need to add additional information around how I addressed the problem

Copy link
Collaborator

@sumitramanga sumitramanga left a comment

Choose a reason for hiding this comment

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

LGTM! 🎸

@miquelbeltran miquelbeltran merged commit e27c74b into develop Apr 29, 2024
3 checks passed
@miquelbeltran miquelbeltran deleted the 148-update-examples branch April 29, 2024 05:33
miquelbeltran added a commit that referenced this pull request May 6, 2024
* express example: upgrade dependencies and point to subfolder raygun package

* using-domains: upgrade dependencies and reference local raygun

* using-domains: upgrade dependencies and reference local raygun

* add console logs and format using spaces

* improved readme

* improved readme + replace tabs by spaces

* enforce changing API keys to run examples
miquelbeltran added a commit that referenced this pull request May 8, 2024
* ci: Create node.js.yml (#142)

* ci: Create node.js.yml

* Include "develop" in the branch list to run CI actions

* trigger GitHub actions.

* fix: Updating dependencies and fixes (#144)

* chore: updated dependencies

* chore: update to tslint-react 5.0.0

* fix: mixup between Request and RequestParams

* fix: convert Express Request to RequestParams

* fix: recreate package-lock to fix express dependency errors

* fix: test tap command update

* fix: test asserts

* refactor: use equal instead of ok

* Adding PR, bug report and feature request templates.

* Update pull_request_template.md

Committing @sumitramanga's suggestion

Co-authored-by: Sumitra Manga <[email protected]>

* Update .github/bug_report.md

Nice!

Co-authored-by: Sumitra Manga <[email protected]>

* Update .github/bug_report.md

Co-authored-by: Sumitra Manga <[email protected]>

* ci: #145 eslint setup (#156)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* chore: #148 upgrade examples (#157)

* express example: upgrade dependencies and point to subfolder raygun package

* using-domains: upgrade dependencies and reference local raygun

* using-domains: upgrade dependencies and reference local raygun

* add console logs and format using spaces

* improved readme

* improved readme + replace tabs by spaces

* enforce changing API keys to run examples

* ci: add PR title checks (#167)

* ci: Update dependabot.yml (#168)

* chore(deps): bump actions/checkout from 3 to 4 (#171)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump actions/setup-node from 3 to 4 (#172)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 3 to 4.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v3...v4)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore: Move pr.yml to workflows (#175)

* refactor: #158 remove no-explicit-any ignore eslint rule (#159)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* update package lock in sample

* Update lib/raygun.sync.worker.ts

Co-authored-by: Sumitra Manga <[email protected]>

* remove empty line

---------

Co-authored-by: Sumitra Manga <[email protected]>

* refactor: #158 remove no-this-alias ignore eslint rule (#160)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* update package lock in sample

* refactor: #158 fix eslint rule "no-unused-vars" (#170)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* setup no-unused-vars lint rule

* remove unused var

* fixed lib/raygun.batch.ts

* fix lib/raygun.offline.ts

* cleanup lib/raygun.sync.transport.ts

* fix lib/raygun.transport.ts

* fix lib/raygun.ts

* fix @typescript-eslint/no-unused-vars

* fixs in test files

* update package lock in sample

* cleanup eslint config

* ci: #174 Prettier setup (#169)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* add examples to prettier command

* add prettier job to node js ci job

* script fix

* setup no-unused-vars lint rule

* remove unused var

* fixed lib/raygun.batch.ts

* fix lib/raygun.offline.ts

* cleanup lib/raygun.sync.transport.ts

* fix lib/raygun.transport.ts

* fix lib/raygun.ts

* fix @typescript-eslint/no-unused-vars

* fixs in test files

* update package lock in sample

* cleanup eslint config

* config files

* run npm run prettier

* Update .github/workflows/node.js.yml

Co-authored-by: Sumitra Manga <[email protected]>

* fix workflow

* add comments to the package.json

* add missing files

* remove fetch-depth: 0

---------

Co-authored-by: Sumitra Manga <[email protected]>

* chore: #158 document why no-var-requires ignore is necessary (#173)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* add examples to prettier command

* add prettier job to node js ci job

* script fix

* setup no-unused-vars lint rule

* remove unused var

* fixed lib/raygun.batch.ts

* fix lib/raygun.offline.ts

* cleanup lib/raygun.sync.transport.ts

* fix lib/raygun.transport.ts

* fix lib/raygun.ts

* fix @typescript-eslint/no-unused-vars

* fixs in test files

* update package lock in sample

* cleanup eslint config

* config files

* run npm run prettier

* chore: document why no-var-requires ignore is necessary

update comment

* refactor: #158 configure no-undef eslint rule  (#176)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* add examples to prettier command

* add prettier job to node js ci job

* script fix

* setup no-unused-vars lint rule

* remove unused var

* fixed lib/raygun.batch.ts

* fix lib/raygun.offline.ts

* cleanup lib/raygun.sync.transport.ts

* fix lib/raygun.transport.ts

* fix lib/raygun.ts

* fix @typescript-eslint/no-unused-vars

* fixs in test files

* update package lock in sample

* cleanup eslint config

* config files

* run npm run prettier

* chore: document why no-var-requires ignore is necessary

update comment

* fix: #158 configure no-undef eslint rule

* refactor: #158 configure no-unreachable eslint rule (#177)

* refactor: #158 remove no-useless-catch eslint rule (#178)

* refactor: #158 configure no-unreachable eslint rule

* refactor: #158 remove no-useless-catch eslint rule

* chore: Prepare for Release 0.14.0 (#179)

* refactor: #158 configure no-unreachable eslint rule

* refactor: #158 remove no-useless-catch eslint rule

* set version 0.14.0

* update changelog

* Update package.json

Co-authored-by: Sumitra Manga <[email protected]>

* add Error reporting to keywords

* update license copyright year

* updated URL

* added more keywords

* moar keywords

---------

Co-authored-by: Sumitra Manga <[email protected]>

* chore: Update package-lock.json (#180)

* chore: update package-lock.json (#183)

* chore: change author name (#185)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Kai Koenig <[email protected]>
Co-authored-by: Sumitra Manga <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

3 participants