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
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 199 | It would be nice to have a trim_trailing_slash helper. | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 175 | 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 | ||
|---|---|---|
| 175 | Fair enough. | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 179–186 | 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 | ||
|---|---|---|
| 179–186 | 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 | ||
|---|---|---|
| 179–186 | 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 | ||
|---|---|---|
| 179–186 | 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 | ||
|---|---|---|
| 179–186 | And what happens if you remove it? | |
| rust/hg-core/src/utils/hg_path.rs | ||
|---|---|---|
| 179–186 | 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 | ||
|---|---|---|
| 195 | 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.