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

Fix: Avoid triggering notification if metadata changes but not the attribute value itself #3727 #4090

Closed
wants to merge 13 commits into from

Conversation

Anjali-NEC
Copy link
Contributor

Fixed issue #3727

root added 6 commits March 16, 2022 06:19
Conflicts:
	src/lib/cache/subCache.cpp
	src/lib/cache/subCache.h
	src/lib/mongoBackend/MongoCommonSubscription.cpp
	src/lib/mongoBackend/MongoCommonSubscription.h
@fgalan
Copy link
Member

fgalan commented Mar 25, 2022

Thanks for your contribution!

As main comment at the present moment: .test cases should be included to cover the functionality developed by this PR.

In addition, note that the PR is breaking some existing .test. That shouldn't happen, except in justified cases (in which case, please provide the proper explanation).

@Anjali-NEC
Copy link
Contributor Author

As main comment at the present moment: .test cases should be included to cover the functionality developed by this PR.

Added test case in https://github.com/telefonicaid/fiware-orion/pull/4090/files#diff-fbc7a73ce9da4fb932ceed7d59dfe48aba94e9aa68b2c633c41bca1e130deb2e

In addition, note that the PR is breaking some existing .test. That shouldn't happen, except in justified cases (in which case, please provide the proper explanation).

I am looking on it.

@fgalan
Copy link
Member

fgalan commented Apr 5, 2022

Thanks for the recent fixes but it seems that there are .test files still failing...

@Anjali-NEC
Copy link
Contributor Author

Thanks for the recent fixes but it seems that there are .test files still failing...

I have tried to fix the failing test cases but I didn't found the suitable solution. Could you please suggest what's changes needs to be done. Thanks

@fgalan
Copy link
Member

fgalan commented Apr 21, 2022

I have tried to fix the failing test cases but I didn't found the suitable solution. Could you please suggest what's changes needs to be done. Thanks

To do so I'd need to do an in deep analysis of the PR, probably downloading your branch in my system and test on it. I'm afraid I cannot do it in the short term...

@fgalan
Copy link
Member

fgalan commented Sep 13, 2022

We shouldn't see this in .test files

imagen

It seems to be a wrongly solved merge conflict.

Please fix it and let's check what the next step is.

@fgalan
Copy link
Member

fgalan commented Dec 19, 2023

After the merging of PR #4332 this PR needs to be upgrades with master branch. Otherwise, functional test will fail.

@Anjali-NEC Anjali-NEC closed this Feb 1, 2024
@fgalan
Copy link
Member

fgalan commented Feb 1, 2024

I understand this PR has been closed, as issue #3737 was finally solved in others PRs.

@Anjali-NEC thanks anyway!

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.

2 participants