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

V7: Metadata refactor #658

Merged
merged 6 commits into from
Dec 3, 2019
Merged

V7: Metadata refactor #658

merged 6 commits into from
Dec 3, 2019

Conversation

bengourley
Copy link
Contributor

  • Rename metaData -> metadata
  • Move the storage to a private property _metadata and remove from the public interface
  • Add add/get/clearMetadata() methods via a shared delegate to Event and Client

@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Nov 29, 2019

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 39.21 kB 11.90 kB
After 39.55 kB 11.96 kB
± ⚠️ +346 bytes ⚠️ +67 bytes

Generated by 🚫 dangerJS against c1efd3b

@bengourley bengourley force-pushed the v7-metadata-refactor branch 2 times, most recently from 0121ff2 to 0673f15 Compare November 29, 2019 15:27
CHANGELOG.md Outdated
@@ -12,6 +12,7 @@
- Update signature of `notify(err, opts?, cb?)` -> `notify(err, onError?, cb?)` for a canonical way to update events [#655](https://github.com/bugsnag/bugsnag-js/pull/655)
- Simplify client configuration, and store resulting config privately [#656](https://github.com/bugsnag/bugsnag-js/pull/656)
- User is now stored privately on `client` and `event` and updated via get/set methods [#657](https://github.com/bugsnag/bugsnag-js/pull/657)
- Rename `metaData` -> `metadata` and add consistent `add/get/clearMetaData()` methods to `Client`/`Event` for manipulating metadata explicitly, rather than mutating a property [#658](https://github.com/bugsnag/bugsnag-js/pull/658)
Copy link
Contributor

Choose a reason for hiding this comment

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

add/get/clearMetaData() -> add/get/clearMetadata()

})
})

describe('event.clearMetadata()', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

describe('event.clearMetadata()' -> describe('event.addMetadata()'

Comment on lines -97 to -105
export interface NotifyOpts {
context?: string;
device?: object;
request?: object;
user?: object;
metaData?: object;
severity?: "info" | "warning" | "error";
onError?: OnError;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this removal intended as part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, it should have happened in #655 – I must have messed up somehow. Since #655 is already merged NotifyOpts is no longer used but I can split it out into a separate PR if you prefer?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can leave it in this PR for me, just wanted to be sure it was right.

packages/core/lib/metadata-delegate.js Outdated Show resolved Hide resolved
packages/core/event.js Show resolved Hide resolved
packages/core/lib/metadata-delegate.js Outdated Show resolved Hide resolved
@bengourley bengourley merged commit 7e70758 into v7 Dec 3, 2019
@bengourley bengourley deleted the v7-metadata-refactor branch December 3, 2019 09:18
@bengourley bengourley mentioned this pull request Apr 14, 2020
Merged
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