-
Notifications
You must be signed in to change notification settings - Fork 185
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
feat: gateway upgrade #87
Conversation
state, | ||
)); | ||
} | ||
let mut uri_path = u.path().to_string(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for to_string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required for the write!
4 lines lower.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
write takes &str happily
query_file_name, | ||
content_path: cpath.to_string(), | ||
content_path: full_content_path.to_string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe store the Path
directly instead of the stringified version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the smaller manipulations require details from the explicit path that was passed in. Some of those details get lost on the resolver::Path
like whether the path had a trailing /
at the end or not. Opening an issue as most of them are able to utilize it properly though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some smaller stuff. Biggest issue for me is missing tests for a lot of logic changes/additions, fine to punt for another PR, but should at least have an issue to start tracking
My next quest is integration testing. Opening an issue to stay on top of it. |
Closes n0-computer/beetle#269