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

Rework #123

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Rework #123

wants to merge 8 commits into from

Conversation

NULLx76
Copy link

@NULLx76 NULLx76 commented Oct 20, 2024

Regarding #121, #122

Mostly done, still expect to make some changes.

  • Currently, the state.json has a different file format, can easily be changed.
  • The data directory is currently hard coded to ./data_new for testing
  • I have not yet ran it long enough to test for the rate limits
  • Add some nicer error messages instead of just using ? everywhere
  • Add a few more doc comments around the place

@NULLx76 NULLx76 force-pushed the refactor branch 3 times, most recently from 4265e85 to dd071a8 Compare October 21, 2024 08:04
src/data.rs Outdated Show resolved Hide resolved
src/data.rs Outdated Show resolved Hide resolved
still need to fix:
- timeout
- csv file header
@NULLx76 NULLx76 changed the title WIP Rework Rework Oct 21, 2024
@NULLx76 NULLx76 marked this pull request as ready for review October 21, 2024 10:35
.cargo/config.toml Outdated Show resolved Hide resolved
@@ -1,27 +1,8 @@
// Copyright (c) 2018 Pietro Albini <pietro@pietroalbini.org>
Copy link
Author

Choose a reason for hiding this comment

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

These copyright notices seemed redundant as there is also a LICENSE file, if we do really want them I can add them back

@NULLx76 NULLx76 requested a review from jdonszelmann October 21, 2024 10:41
@NULLx76
Copy link
Author

NULLx76 commented Oct 25, 2024

image
This shows the timeouts being correctly handled

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