-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
semver validate exit codes #71
Comments
(I'm sure that I, somewhere, wrote a comment about this sort of change; but, for the life of me, I can't find it. So, here it goes again) As of today, the semver-tool communicates results on
One advantage of this scheme is that non-zero return codes mean there is an error and exceptional processing can be triggered. For CI pipelines, this might be aborting the build: the pipe itself is broken. And (for those who don't write CI pipeline tests!), coding errors don't get "swallowed". E.g. misspell "validate" and suddenly all versions are "invalid". Also, the code above makes the logic explicit. Now, there are other points of view. See, for example, the man page for cmp. Also, see the @fsaintjacques comment to issue #61. While the comment suggests using the return value, this was not implemented and the subsequent PR #62 was accepted :-). So, I see (for now), a few questions to discuss and resolve:
|
If the version isn't valid, I do indeed want to break the build. I can of course script this in my pipeline (or Bazel, or Make) but then I might as well just use the regex you provide and I don't need semver at all. I'd love to move away from the NPM provided semver (for about 100 reasons) but why not make this tool more like other *nix tools? |
Yes, it could be that an invalid version string indicates a fatal error/bug in the build system. It could also mean, for example, that some versions (some libraries, legacy, ..) of software are not using semantic versioning and need to be handled differently. As for being more *nix-like. Well, it's not clear. Searching around, I found a fair amount of information about standardized exit code values (e.g. https://tldp.org/LDP/abs/html/exitcodes.html). The common usage is for errors, not results; but there are exceptions (e.g. "cmp"). When errors and results are mixed, it pretty much becomes necessary to have some sort of explicit test to distinguish between an error (e.g. for "file not found", cmp returns "2") and a result (cmp returns "1" if file contents differ). For me, it's hard to see how things become shorter or simpler. I still think that the questions posed above make sense and anyone motivated to provide a PR should address them. |
@ranger6 - Agreed and I certainly didn't mean to imply that my use-case was THE use- case :) I also wasn't recommending this behavior for the other functions this program provides and was rather suggesting that failing a version check is indeed an error. I'm only using this to make sure that when our developers try to tag a release/pre-release, they've followed semantic versioning and while it kills me a little on the inside, I can keep validating using the NPM package. |
No, I'm sure that you were not claiming your use case as "the way". And, adding an error exit code to "validate" does not necessarily imply adding it to other sub-commands: as I mentioned, it "opens the door". If the model for validate is to drive logic from exit values rather than stdout values, then why not also (for example) for "compare"? But as I understand your (reasonable) point of view: "failing a version check is indeed an error". In your use case (someone not using semantic versioning as required), it is an error .. at the level of your build system logic. For my example use case (support for legacy code), it's not an error. I would use the result (stdout or exit code) for branching logic. I would not want or expect the command to raise an exception that I'd catch to abort the build. In fact, if a real error condition was present (e.g. command not found because the PATH not set correctly), I would like a (fatal) error raised (and maybe caught/handled in an exceptional manner). As a practical matter, in Bash, the real error conditions get swallowed when they occur in "if", "while", "test" structures. So, it is hard to distinguish errors from non-semantic versions .. in Bash. The good news and argument for going forward with the exit code proposal is that I believe there may not be so many situations where the logic would be broken (in Bash scripts). Mostly because the use of the "validate" sub-command as it operates today will mostly be in conditional expressions such as:
Above, if "validate" is mis-spelled, or Now the bad news:
If one adds a non-zero exit code to "validate", an error will be raised. If I'm catching errors (e.g. To finish up: semver does consider a non-semantic version string to be an error when the sub-command expects a valid semantic version string. E.g.
P.S. Thanks for the engagement! |
Hi community,
I'm raising an issue to see if anyone else would be interested in having exit codes returned when using the 'validate' command.
Since this tool is built with CI in mind, it would be useful if a pipeline, pre-commit hook, etc. could be failed due to a non-zero exit code on version validation, rather than just echoing valid/invalid.
What does everyone else think? Is there some gotcha that I didn't consider? Maybe it could be controlled with a flag to start with, e.g. --exit-code, so as to not break pipelines currently using this tool.
Thanks for your consideration
The text was updated successfully, but these errors were encountered: