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 circe source code to checksum library #44

Merged
merged 1 commit into from
Sep 29, 2016

Conversation

rdhabalia
Copy link
Contributor

Motivation

Pulsar-checksum uses circe library to compute checksum but it's release-binaries are not available. So, to avoid depending on snapshot version, import the source code of this library.

Modifications

Import source classes from circe repo which requires to compute crc32c checksum.

Result

No functionality change. However, while building pulsar, it doesn't have to download circe binaries.

@yahoocla
Copy link

CLA is valid!

Hash Algorithm Framework & Library, which can be obtained at:

* LICENSE:
* licenses/LICENSE-2.0 (Apache License)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@merlimat : should we add any additional information under LICENSE.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can just mention :

LICENSE : Apache License, Version 2.0

@rdhabalia rdhabalia added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Sep 28, 2016
@rdhabalia rdhabalia added this to the 1.15 milestone Sep 28, 2016
@rdhabalia rdhabalia self-assigned this Sep 28, 2016
@@ -26,3 +26,12 @@ Google's data interchange format, which can be obtained at:
* license/LICENSE.protobuf.txt (New BSD License)
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually.. this file we don't have it there :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. So, should we change it to LICENSE : Google Inc. or we can keep it as it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should add the protobuf license file as part of a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added protobuf/LICENSE.protobuf.txt

Hash Algorithm Framework & Library, which can be obtained at:

* LICENSE:
* licenses/LICENSE-2.0 (Apache License)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can just mention :

LICENSE : Apache License, Version 2.0


* LICENSE:
* licenses/LICENSE-2.0 (Apache License)
* Git:
Copy link
Contributor

Choose a reason for hiding this comment

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

HOMEPAGE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. changed to:

@merlimat
Copy link
Contributor

I think we should also import unit tests

@rdhabalia
Copy link
Contributor Author

imported unit-test cases after converting from junit to testng.

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

Thanks for converting all unit tests!

<artifactId>circe-crc</artifactId>
<version>1.1-20140603.044857-1</version>
<groupId>com.googlecode.jmockit</groupId>
<artifactId>jmockit</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be added to test scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes.. i missed one commit to push.. it's only required for test so, scope should be test. made the change.

@merlimat merlimat merged commit 554f050 into apache:master Sep 29, 2016
@rdhabalia rdhabalia deleted the cierce branch November 11, 2016 23:01
sijie added a commit to sijie/pulsar that referenced this pull request Mar 4, 2018
* Introduce FunctionStats to collection function related stats

* Format license headers
massakam pushed a commit to massakam/pulsar that referenced this pull request Aug 6, 2020
hangc0276 pushed a commit to hangc0276/pulsar that referenced this pull request May 26, 2021
In Pulsar repo, there was already a Protocol handler framework, This PR tries build KOP in a nar file, and to make KoP.nar runs as a Protocol handler for Pulsar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants