Validate dependencies and incompatibilities #91
Conversation
ithinkandicode
changed the title
| var intro_text = "A %s for the mod '%s' is invalid: " % [type, original_mod_id] | ||
|
|
||
| # contains hyphen? | ||
| if not check_mod_id.count("-") == 1: |
There was a problem hiding this comment.
you could use the is_name_or_namespace_valid func if you String.split("-") here (and checked if the array length is exactly 2)
There was a problem hiding this comment.
I tried this originally, but it wasn't nearly as helpful. Here's the error if I use is_name_or_namespace_valid:
And here's the error message with the approach I've used:
The 1st error tells you something is wrong with either a name or a namespace, but gives no indication of where/how to fix the problem. You don't know if it's occurring in your own mod or a dependency of it, nor whether it's an issue with the mods own name, or a dependency, or an incompatibility.
Whereas the 2nd error tells you which mod has an issue, and exactly where the issue is.
Trying to use is_name_or_namespace_valid initially is actually the reason I made my errors so explicitly helpful: When I was testing this I made a bunch of purposefully faulty manifests, but found that I had no idea which of them were incorrect.
Better to do as much as we can to help people fix issues like this IMO.
There was a problem hiding this comment.
Hm, you are right. Then it might be a better idea to turn the log in is_name_or_namespace_valid into an error and add the log fatal like you did here once it returns false. I think that avoids having two places for id validation and the logs are still as informative, right? And don't forget to check false and log fatal after the other uses of it.
There was a problem hiding this comment.
Maybe that would be better in a subsequent PR? I don't want to change anything beyond the scope of validating deps/incompats for this one
There was a problem hiding this comment.
to get it done with I suppose. Having two places for the same validation is not good though, that needs to change
…, and make the `type` arg optional
ithinkandicode
deleted the
validate-dependencies-and-incompatibilities
branch
Validates dependencies and incompatibilities, checking the following for any specified mod IDs:
-Closes #23