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

Update KffFile to use FileChannelFactory instead of RandomAccessFile #527

Merged
merged 6 commits into from
Sep 5, 2023
Merged

Update KffFile to use FileChannelFactory instead of RandomAccessFile #527

merged 6 commits into from
Sep 5, 2023

Conversation

drivenflywheel
Copy link
Collaborator

@drivenflywheel drivenflywheel commented Jul 31, 2023 •
edited
Loading

Addresses an existing thread safety issue.

@drivenflywheel drivenflywheel marked this pull request as ready for review August 3, 2023 16:54
@drivenflywheel drivenflywheel added the bug Something isn't working as intended label Aug 3, 2023
@drivenflywheel drivenflywheel added needs review Reviews and approvals are still needed and removed bug Something isn't working as intended labels Aug 11, 2023
jdcove2
jdcove2 reviewed Aug 16, 2023
View reviewed changes
Copy link
Collaborator

@jdcove2 jdcove2 left a comment

Choose a reason for hiding this comment

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

The 3-argument constructor should probably have "DEFAULT_RECORD_LENGTH" instead of "24" in it's "this" call.

@drivenflywheel
Copy link
Collaborator Author

The 3-argument constructor should probably have "DEFAULT_RECORD_LENGTH" instead of "24" in it's "this" call.

Done

fbruton
fbruton previously approved these changes Aug 23, 2023
View reviewed changes
src/test/java/emissary/kff/KffFileTest.java Outdated Show resolved Hide resolved
jdcove2
jdcove2 previously approved these changes Aug 23, 2023
View reviewed changes
@jpdahlke jpdahlke added this to the v8.0.0-M4 milestone Aug 24, 2023
@drivenflywheel drivenflywheel dismissed stale reviews from jdcove2 and fbruton via c9f12a5 August 28, 2023 19:59
jdcove2
jdcove2 previously approved these changes Aug 29, 2023
View reviewed changes
@jpdahlke jpdahlke requested a review from fbruton August 29, 2023 13:54
@cfkoehler cfkoehler self-requested a review September 1, 2023 09:25
cfkoehler
cfkoehler previously approved these changes Sep 1, 2023
View reviewed changes
src/main/java/emissary/kff/KffFile.java Outdated Show resolved Hide resolved
@cfkoehler cfkoehler removed the needs review Reviews and approvals are still needed label Sep 1, 2023
@jpdahlke jpdahlke merged commit aacc0fc into NationalSecurityAgency:master Sep 5, 2023
8 of 9 checks passed
@jpdahlke jpdahlke added enhancement An enhancement or update to an existing feature bug Something isn't working as intended labels Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbruton fbruton fbruton approved these changes

@cfkoehler cfkoehler cfkoehler approved these changes

@jdcove2 jdcove2 Awaiting requested review from jdcove2

Assignees
No one assigned
Labels
bug Something isn't working as intended enhancement An enhancement or update to an existing feature
Projects
None yet
Milestone
v8.0.0-M4
Development

Successfully merging this pull request may close these issues.
5 participants
@drivenflywheel @fbruton @jdcove2 @cfkoehler @jpdahlke