-
Notifications
You must be signed in to change notification settings - Fork 41
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(aggregator): better url sanitization process by using a value object #2241
Conversation
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.
LGTM 👍
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.
LGTM
It's a good thing to have a typed Url but the name should be improve
internal_url: Url, | ||
} | ||
|
||
impl SanitizedUrl { |
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.
This structure represent a Uri that is not a resource.
We may named it to reflect that.
SanitizedUrl
doesn't mean much. This gives the impression of having a valid Url
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.
We share this concern but could not find a name that would fit that description.
Since we could not find anything better we renamed it to SanitizedUrlWithTrailingSlash
with hope we found a better name in the future.
b630878
to
0db35ae
Compare
33f7afc
to
9db66d8
Compare
9db66d8
to
18e7535
Compare
…e object This `SanitizedUrlWithTrailingSlash` struct allow to avoid redundant call to the url sanitizer as it guarantee that the url that it hold is already sanitized. Co-authored-by: Damien Lachaume <[email protected]>
* mithril-aggregator from `0.6.18` to `0.6.19`
18e7535
to
d65c60a
Compare
Content
This PR introduce a
SanitizedUrlWithTrailingSlash
struct to avoid redundant call to the url sanitizer as it guarantee that the url that it hold is already sanitized.Pre-submit checklist