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

[Kraken] fix erase vj from dataset #3910

Merged
merged 1 commit into from
Feb 13, 2023
Merged

[Kraken] fix erase vj from dataset #3910

merged 1 commit into from
Feb 13, 2023

Conversation

xlqian
Copy link
Member

@xlqian xlqian commented Jan 13, 2023

fix "impossible to find the vj" when erasing vj from dataset

@xlqian xlqian marked this pull request as ready for review January 13, 2023 14:40
@xlqian xlqian force-pushed the fix_erase_vj_from_dataset branch from 25917c3 to 712d85d Compare February 6, 2023 12:40
@xlqian xlqian force-pushed the fix_erase_vj_from_dataset branch from 712d85d to 118c2d3 Compare February 7, 2023 16:53
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 7, 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 2 Code Smells

76.5% 76.5% Coverage
0.0% 0.0% Duplication

@xlqian xlqian requested a review from pbougue February 7, 2023 21:28
Comment on lines +415 to +416
if (!pt_data.datasets.empty()) {
if (pt_data.datasets[0]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you're checking almost twice the same thing, but if you want to keep double check it's probably more expressive like:

Suggested change
if (!pt_data.datasets.empty()) {
if (pt_data.datasets[0]) {
if (!pt_data.datasets.empty() && pt_data.datasets[0] != nullptr) {

@xlqian xlqian merged commit 34df7ae into dev Feb 13, 2023
@xlqian xlqian deleted the fix_erase_vj_from_dataset branch February 13, 2023 15:13
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