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

Replace rocket to url for URI parsing #484

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

2e3s
Copy link
Contributor

@2e3s 2e3s commented Jun 11, 2024

Url crate is most popular and basic for the task.
Rocket version in aw-transform is also out of sync with aw-server, which may cause a conflict elsewhere.


🚀 This description was created by Ellipsis for commit 1148540

Summary:

Replaced rocket crate with url for URI parsing in aw-transform to resolve version conflicts and use a more popular crate.

Key points:

  • Replaced rocket crate with url crate in aw-transform.
  • Updated Cargo.toml and split_url.rs to use url for URI parsing.
  • Resolved version conflicts between aw-transform and aw-server.
  • Improved URI parsing by using a more popular and basic crate.

Generated with ❤️ by ellipsis.dev

Url crate is most popular and basic for the task.
Rocket version in aw-transform is also out of sync with aw-server,
which may cause a conflict elsewhere.
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 1148540 in 1 minute and 8 seconds

More details
  • Looked at 474 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. aw-transform/src/split_url.rs:44
  • Draft comment:
    The domain extraction logic removes the "www." prefix, which is generally a good approach for normalizing domain names. However, this might not be desired in all cases, and it could be beneficial to make this behavior configurable or clearly document it.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The PR description mentions that the url crate is being used because it is more popular and basic for the task, and there is a version conflict with rocket in aw-transform and aw-server. The changes in the Cargo.toml and Cargo.lock reflect this update by removing rocket and adding url along with its dependencies. The code changes in split_url.rs replace the rocket URI parsing with url crate parsing. The logic for extracting components from the URL (protocol, domain, path, params) is correctly updated to use the methods provided by the url crate.

Workflow ID: wflow_p1eYTXo2pBBycALz


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Reqwest specifies a particular version which may be a problem.
@ErikBjare
Copy link
Member

Nice, thanks!

@ErikBjare ErikBjare merged commit bb787fd into ActivityWatch:master Jun 11, 2024
6 of 7 checks passed
@2e3s 2e3s deleted the rocket-transform-upgrade branch June 11, 2024 17:04
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