Dave Reisner <d...@falconindy.com> wrote: > I didn't go over this in detail, but I'll point out a few concerns I > have about this patch...
Fair enough, thanks for the feedback. > On Fri, Sep 15, 2017 at 01:30:51PM -0500, ivy.fos...@gmail.com wrote: > > +Errors > > +------ > > +On exit, makepkg will return one of the following error codes. > > + > > +**E_OK**=0:: > > I don't see the need to document internal details of how we refer to the > error codes through named variables if we aren't making these public API. The rationale here was that it could be useful information for anybody scripting builds, but I don't feel strongly about it. I do see your point; anybody using these to (say) make tests for makepkg can easily figure out what they all mean from the names in the source. > > +**E_BUILD_FAILED**=5:: > > + Error in PKGBUILD build function. > > + > > +**E_PACKAGE_FAILED**=6:: > > + Failed to create a viable package. > > What about failures in the prepare of pkgver functions (and any future > functions which haven't yet been defined)? I think this ought to be more > generic and be something like E_USER_FUNCTION_FAILED. That's a good idea. > > +**E_PRETTY_BAD_PRIVACY**=17:: > > + Error signing package. > > As implemented, this is only used when checking to see that the key > exists, not as a failure when signing the package. To do that, you'd > need to change scripts/libmakepkg/integrity/generate_signature.sh.in, > and then make sure the error code is propagated down the stack. ...you're quite right. Will fix. > > +# exit code 2 reserved by bash for misuse of shell builtins > > Not sure I understand this... When would this clash? I'm not sure that it would; I just happened across that tidbit in tldp's bash scripting guide while looking for something else. Further research (actually looking it up in bash(1)) reveals that it isn't actually *reserved*, just used for that by bash. Will fix. Thanks again for the critique. I'll get this stuff cleaned up sometime in the next couple of days. iff