You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "raboof (via GitHub)" <gi...@apache.org> on 2024/07/02 16:59:42 UTC
[PR] Simplify UriParser [commons-vfs]
raboof opened a new pull request, #558:
URL: https://github.com/apache/commons-vfs/pull/558
The reverts https://github.com/apache/commons-vfs/pull/396 and related
changes and implements the same in a simpler way by replacing the
encoded characters already in `fixSeparators`.
This approach has a slightly higher risk at breaking existing behaviour,
but a lower risk of remaining problems in this part of the codebase. All
testcases still succeed.
This PR is intended to replace https://github.com/apache/commons-vfs/pull/543 and https://github.com/apache/commons-vfs/pull/555. It includes the testcases
from https://github.com/apache/commons-vfs/pull/543, adapted to the behaviour before https://github.com/apache/commons-vfs/pull/396.
This reverts commit https://github.com/apache/commons-vfs/commit/cb45c9453669eb2570f582f491cb0e06b0e63466.
This reverts commit https://github.com/apache/commons-vfs/commit/5399c766287a06ce002beaed89fa8af5461e15d0.
This seems only slightly (<8%) slower than #547 , which might be acceptable as it's not obvious this is a bottleneck:
```
2290973,673 ±(99.9%) 55810,967 ops/s [Average]
2123414,134 ±(99.9%) 48374,989 ops/s [Average]
```
Should probably add some more tests exercising the problems that remain after the fixes from #543
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify UriParser [commons-vfs]
Posted by "japplis (via GitHub)" <gi...@apache.org>.
japplis commented on PR #558:
URL: https://github.com/apache/commons-vfs/pull/558#issuecomment-2243360117
I also think it's OK to merge.
Small remark StringBuilder#deleteCharAt could have been used at line 533 & 568 of UriParser.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify UriParser [commons-vfs]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #558:
URL: https://github.com/apache/commons-vfs/pull/558#issuecomment-2252123751
Yes: I just fixed the merge conflict with 95ddecdfd55338ff3efdf8a6dda24fc10d2425d9 and switched to `StringBuilder#deleteCharAt` for the two instances @japplis mentioned.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify UriParser [commons-vfs]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #558:
URL: https://github.com/apache/commons-vfs/pull/558#issuecomment-2251552833
@raboof
Good to merge?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify UriParser [commons-vfs]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #558:
URL: https://github.com/apache/commons-vfs/pull/558#issuecomment-2241804433
I'd be OK to merge. Anyone else?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify UriParser [commons-vfs]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #558:
URL: https://github.com/apache/commons-vfs/pull/558#issuecomment-2243473689
@japplis I'm more concerned about the semantics than implementation details at this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify UriParser [commons-vfs]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #558:
URL: https://github.com/apache/commons-vfs/pull/558
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org