Code review comment for lp:~jelmer/bzr-upload/only-strip-slash

Revision history for this message
Martin Packman (gz) wrote :

Only stripping trailing slashes is clearly an improvement on always removing whatever the last character is and this should clearly land.

However, doing url manipulation with basic string functions, even simple stuff like this, makes me nervous. I'm thinking particularly of this change:

<https://code.launchpad.net/~gz/bzr/root_drive_file_url_841322/+merge/74034>

         self.base = base
- self._segment_parameters = urlutils.split_segment_parameters(
- base.rstrip("/"))[1]
+ self._segment_parameters = urlutils.split_segment_parameters(base)[1]

Which was needed to avoid some very surprising breakage.

review: Approve

« Back to merge proposal