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.

Sign up for GitHub

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

Jump to bottom

Cleared TODO regarding segfaults and fixed re-publishing metadata behavior #71

Merged
merged 7 commits into from
Aug 15, 2024
Merged

Cleared TODO regarding segfaults and fixed re-publishing metadata behavior #71

merged 7 commits into from
Aug 15, 2024

Conversation

BrianMichell
Copy link
Collaborator

Resolves #46

Addressed errant memory access in Dataset CommitMetadata call.

  • Fixed Stack-Use-After-Return issue
  • Fixed Heap-Use-After-Free issue
  • Added tracking of changed metadata back so publish can be more efficient
  • Expanded test case coverage

@BrianMichell BrianMichell added the bug Something isn't working label Aug 5, 2024
@BrianMichell BrianMichell requested a review from blasscoc August 5, 2024 19:51
blasscoc
blasscoc reviewed Aug 7, 2024
View reviewed changes
Copy link
Collaborator

@blasscoc blasscoc left a comment

Choose a reason for hiding this comment

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

 How robust is our testing here?

@BrianMichell
Copy link
Collaborator Author

BrianMichell commented Aug 7, 2024 •
edited
Loading 

 How robust is our testing here?

It could always stand to be more robust. Currently we have testing for deleting local datasets and GCS datasets.
Testing here is actually quite robust. I got the PRs mixed up in my head here.

@blasscoc
Copy link
Collaborator

Test run LGTM

blasscoc
blasscoc approved these changes Aug 15, 2024
View reviewed changes
Copy link
Collaborator

@blasscoc blasscoc left a comment

Choose a reason for hiding this comment

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

LGTM

@blasscoc blasscoc merged commit 2d14001 into main Aug 15, 2024
6 checks passed
@blasscoc blasscoc deleted the 46_publish_bug branch August 15, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blasscoc blasscoc blasscoc approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TODO - Variable.h
2 participants
@BrianMichell @blasscoc