Closed brodycj closed 6 years ago
I do not understand the fuss being made about this non-issue! :tired_face:
The old implementation was broken in so many ways that this bug is quite a joke. That's why I rewrote almost the whole module. If there's bugs in the new implementation, then certainly not this one that's very specific to the old implementation.
Why don't we just focus on moving forward instead of discussing in four different issues/PRs how we should test and patch broken code that has already been completely removed from master
?
IMHO this is a wontfix
.
I agree, every attempted fix to this is going to cause other weird bugs to surface. Let's just leave it as-is, where we know what's broken, and focus on moving forward.
Closing then. I will probably want to add some more module ID testing in the master branch, or at least raise a PR for consideration, sometime later.
From review of PR #38 (
1.3.1
patch release updates on1.3.x
branch) and unit testing in WIP PR #39 we discovered that the code will use incorrect module ID in case of certain URL patterns on1.3.1
. It is not yet certain whether or not this would be an issue onmaster
.Sample URLs where the incorrect module ID is observed in the unit tests in WIP PR #39:
https://scm.git.service.io/user/my-repo.git
git://scm.git.service.io/user/my-repo.git
https://scm.service.io/user/my-repo-other-url
wherenpm
indicates that it installedmy-repo@2.0.0
https://scm.service.io/user/my-repo#old-tag
git://scm.service.io/user/my-repo#old-tag
Unfortunately the unit tests in WIP PR #39 will not work on
master
due to changes in the code.TODO:
master
master
if needed1.3.x
, before next major release is published from the changes inmaster