This changeset introduces the use of the pretty_assertions crate for easier
to read test output.
Details
Details
Diff Detail
Diff Detail
- Repository
- rHG Mercurial
- Branch
- default
- Lint
No Linters Available - Unit
No Unit Test Coverage
Event Timeline
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 189 | It would be nice to have a trim_trailing_slash helper. | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 169 | I don't think we should have this method. It provides us no way to ensure that the invariants of this type are upheld. It is slightly more typing but I think we should recommend converting to a slice, doing the mutation, then converting back to HgPath then we can at least to validation (possibly only in debug builds) in the constructor. | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 169 | Fair enough. | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 176 | When do we create such paths? I can imagine using them for referring to directories. Is that it? If it's some other use-case, I wonder if we can instead prevent the trailing slash from entering the HgPath. | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 176 | Maybe, but I'm not sure it's worth risking a weird edge-case bug or losing performance over (possibly negligible). What do you think? | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 176 | Another way of thinking about my question: what happens if we never call this function? Which tests fail? | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 176 | This is currently this only place it's called (line 189), it was refactored for readability first I suppose. | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 176 | And what happens if you remove it? | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 176 | It only breaks the single rust test I had to check for that particular case. I'll remove it. | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 185 | Might be good to add an assert! here in case my assertion (that HgPath should never have a trailing /) is not correct. | |
I don't think we should have this method. It provides us no way to ensure that the invariants of this type are upheld. It is slightly more typing but I think we should recommend converting to a slice, doing the mutation, then converting back to HgPath then we can at least to validation (possibly only in debug builds) in the constructor.