Incorrect module ID in case of certain URL patterns

[brodycj]

From review of PR #38 (1.3.1 patch release updates on 1.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 on 1.3.1. It is not yet certain whether or not this would be an issue on master.

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 where npm indicates that it installed my-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:

• add unit testing to cover this behavior in master
• resolve on master if needed
• determine whether or not there is any need to resolve this issue on 1.3.x, before next major release is published from the changes in master

[raphinesse]

I do not understand the fuss being made about this non-issue! 😫

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.

[dpogue]

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.

[brodycj]

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.