Skip to content

feat(metadata)!: remove Notifications config#4320

Merged
cloudxxx8 merged 1 commit intoedgexfoundry:mainfrom
chr1shung:issue-4316
Feb 7, 2023
Merged

feat(metadata)!: remove Notifications config#4320
cloudxxx8 merged 1 commit intoedgexfoundry:mainfrom
chr1shung:issue-4316

Conversation

@chr1shung
Copy link

@chr1shung chr1shung commented Feb 3, 2023 •
edited
Loading

BREAKING CHANGE: Removed the 'Notifications' config and 'Clients.support-notifications' dependency

in EdgeX 3.0, metadata will leverage device system events to replace the original device change notifications

closes #4316

Signed-off-by: Chris Hung chris@iotechsys.com

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)
    feat: remove Notifications configuration in Metadata edgex-docs#954

Testing Instructions

New Dependency Instructions (If applicable)

@chr1shung chr1shung changed the title feat!(metadata): remove Notifications config feat(metadata)!: remove Notifications config Feb 3, 2023
@chr1shung
Copy link
Author

chr1shung commented Feb 3, 2023

The function call and code is removed in #4269 so we only removed the config in this PR.
Also might need to update edgex-compose to remove the notifications dependency on metadata ?

@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2023 •
edited
Loading

Codecov Report

Merging #4320 (372c471) into main (673422c) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main    #4320   +/-   ##
=======================================
  Coverage   43.15%   43.15%           
=======================================
  Files         116      116           
  Lines       10661    10661           
=======================================
  Hits         4601     4601           
  Misses       5656     5656           
  Partials      404      404

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@cloudxxx8
Copy link
Member

cloudxxx8 commented Feb 3, 2023

yes, edgex-compose should also be updated

in addition, please update the document to remove the related config
https://docs.edgexfoundry.org/2.3/microservices/core/metadata/Ch-Metadata/#configuration-properties

cloudxxx8
cloudxxx8 previously approved these changes Feb 3, 2023
View reviewed changes
Copy link
Member

@cloudxxx8 cloudxxx8 left a comment

Choose a reason for hiding this comment

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

LGTM

feat(metadata)!: remove Notifications config
372c471 
BREAKING CHANGE: Removed the 'Notifications' config and
'Clients.support-notifications' dependency

in EdgeX 3.0, metadata will leverage device system events to replace
the original device change notifications

closes #4316

Signed-off-by: Chris Hung <chris@iotechsys.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@chr1shung
Copy link
Author

chr1shung commented Feb 3, 2023

edgex-docs PR: edgexfoundry/edgex-docs#954

@cloudxxx8
Copy link
Member

cloudxxx8 commented Feb 6, 2023

recheck

lenny-goodell
lenny-goodell approved these changes Feb 7, 2023
View reviewed changes
Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

LGTM

@cloudxxx8 cloudxxx8 merged commit c283a7b into edgexfoundry:main Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lenny-goodell lenny-goodell lenny-goodell approved these changes

@cloudxxx8 cloudxxx8 cloudxxx8 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[EdgeX 3.0] Remove the Device change notification calls

4 participants

@chr1shung @codecov-commenter @cloudxxx8 @lenny-goodell