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

Add support for com.github.luben:zstd-jni:1.5.2-5 #121

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

linghengqian
Copy link
Contributor

@linghengqian linghengqian commented Nov 21, 2022

What does this PR do?

Checklist before merging

  • I have properly formatted metadata files (see CONTRIBUTING document)
  • I have added thorough tests. (see this)

Sorry, something went wrong.

@luben
Copy link
Contributor

luben commented Nov 22, 2022

LGTM!

@linghengqian linghengqian force-pushed the zstd-jni branch 3 times, most recently from 3af6c15 to 04375b0 Compare December 5, 2022 15:34
@linghengqian linghengqian mentioned this pull request Dec 9, 2022
2 tasks
@Test
void testCompressFile() throws IOException {
File file = new File("src/test/resources/originTest.txt");
File outFile = new File("src/test/resources/originTest.txt.zs");
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be worth writing these files to a temporary directory so that they don't pollute the main repository

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Do you mean let me replace it with java.io.File.createTempFile, and replace the tail assertDoesNotThrow(outFile::delete); with assertDoesNotThrow(outFile::deleteOnExit);? Do I need to keep committing to this PR or?

Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly. I would commit the change to this PR :)

Copy link
Member

Choose a reason for hiding this comment

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

Is this done?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this done?

@gradinac Any updates?

@vjovanov
Copy link
Member

Could we also remove the merge commit from the history?

@linghengqian
Copy link
Contributor Author

  • @vjovanov Hi, I want to make sure if the current repository allows to directly click on Github's Squash and Merge button? I'm currently away from my computer and can't do something like this right now.
  • image

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@linghengqian
Copy link
Contributor Author

Could we also remove the merge commit from the history?

  • Finish. I'm not sure why Github Actions has so many jobs open.

@vjovanov
Copy link
Member

Looks good now, only one commit.

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.

Add support for com.github.luben:zstd-jni:1.5.2-5
4 participants