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

Refactor Kitty completely #60

Merged
merged 41 commits into from
Feb 25, 2023
Merged

Refactor Kitty completely #60

merged 41 commits into from
Feb 25, 2023

Conversation

avborup
Copy link
Owner

@avborup avborup commented Feb 1, 2023

Goals

I had a few main goals with this refactor:

  • Improve the code quality to make it more readable, extendable, and maintainable
  • Add some sort of integration tests to ensure that core functionality works
  • Improve error messages and ability to diagnose bugs
  • Fix bugs caused by the new Kattis UI update (which broke web scraping)

My main motivation for the refactor is that I feel like I have improved a lot at Rust in the past two years - and I wanted to apply that to kitty. As you can see, the first thing I did was remove all the code in the project so I could go through it, slowly adding/rewriting everything step by step.

What's changed?

As for what has actually changed in the code, this list can be seen as a rough summary:

  • The CLI is now handled structopt-style with clap's derive macros. This makes handling of arguments more type-safe, and the CLI definition file is now far more readable.
  • Everything is passed via arguments to functions, getting rid of the global lazy_static config object. The new App type is central here.
  • Use eyre for error handling, heavily utilising wrap_err (analagous to anyhow's context, if you're familiar with that) to provide specific error messages. Combined with --verbose showing a backtrace, this allows for tracking down a problem more easily.
  • Add integration tests that make sure that the most important happy and unhappy paths have the expected side effects and terminal output.
  • macOS ARM now also receives a prebuilt binary for each new release (aarch64-apple-darwin)
  • The history and random commands have been removed. These commands were seemingly not used often, and it streamlines the purpose of kitty a bit more. If there is a need for them, we can add them back down the line.

Subcommand progress

Tracks how many of the current subcommands have been refactored/reintroduced.

  • config
  • get
  • help
  • history
  • langs
  • open
  • random
  • submit
  • test
  • update

@avborup avborup linked an issue Feb 1, 2023 that may be closed by this pull request
@avborup avborup linked an issue Feb 5, 2023 that may be closed by this pull request
Includes:
* Making everything async
* Dependencies for requests
* Extending languages
* Tests
* Templates
* etc..
@avborup
Copy link
Owner Author

avborup commented Feb 15, 2023

Task failed successfully:
image

Edit: has been fixed

@avborup avborup self-assigned this Feb 16, 2023
@JoachimBorup
Copy link
Contributor

Submitting a Java solution produces the following behaviour:

$ kitty submit addingwords -y
Problem:  addingwords
Language: Java
File:     Addingwords.java
Error: Failed to find submission ID in response from Kattis. Received: No mainclass set.

@avborup avborup linked an issue Feb 22, 2023 that may be closed by this pull request
@avborup avborup marked this pull request as ready for review February 25, 2023 14:37
@avborup avborup merged commit 15ddc0f into master Feb 25, 2023
@avborup avborup deleted the chore/refactor branch February 25, 2023 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants