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

Read and process file line by line #1

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

petricavalry
Copy link

@petricavalry petricavalry commented Nov 5, 2024

Rewrite signature counter.

Some Specific Rules

Signature count was considered zero when missing signature block.

Learn more rules from unit tests.

Breaking Changes

HTML comments should be placed on standalone lines.

Ok:

<!-- BEGIN LGBT-CN SIGNATURE -->
<!-- END LGBT-CN SIGNATURE -->

<!-- BEGIN LGBT-CN COUNT -->
<!-- END LGBT-CN COUNT -->

Wrong:

<!-- BEGIN LGBT-CN SIGNATURE --><!-- END LGBT-CN SIGNATURE -->
<!-- BEGIN LGBT-CN COUNT --><!-- END LGBT-CN COUNT -->

Thanks for all amazing work in LGBTQIA In China.

Feel free to adjust any part of this pull request.

@petricavalry petricavalry marked this pull request as draft November 5, 2024 14:45
@petricavalry petricavalry force-pushed the push-ylvuxkxlorrp branch 2 times, most recently from f001a65 to 57fcc52 Compare November 6, 2024 11:16
@petricavalry petricavalry marked this pull request as ready for review November 6, 2024 11:17
Copying is an art of love. Please copy and share.
@petricavalry petricavalry marked this pull request as draft November 6, 2024 18:04
@petricavalry petricavalry marked this pull request as ready for review November 6, 2024 18:11
@KevinZonda
Copy link
Member

Hi, thank you for your PR. The LGBT-CN SC is designed for a trust box rather than a black box, i.e., we have 2 foundamental assumptions:

  • begin's index < end's index (guaranteeded by CR)
  • low amount data (i.e., the whole data we proccess can fit into the main memory, in other words, work w/o buffer reader is acceptable)

If you dont mind, please explain what this PR can improve, this will help. Thanks again for your PR.

@petricavalry
Copy link
Author

Thanks for your review. I am happy to draft this pull request.

You are right. I have include unnecessary checks and use an unnecessary buffer. It may work better without these changes.

I reviewed the commit history and I have a question: Why did you rewrite the signature-counter (Rust) in Go?

@KevinZonda
Copy link
Member

I am not familiar with Rust. We met an unexpected behaviour with the previous SC (Signature Counter), and I cannot debug the programme with 瞪眼法. So we decided to convert it to Go with a much reliable approach (reliable means if not works, it will panic or broken, which will fail the CI).

@KevinZonda
Copy link
Member

I will draft this PR for a while. I'll rethink this PR when Im free.

@KevinZonda KevinZonda marked this pull request as draft November 15, 2024 03:33
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