Differential D9889
security/krb5-115: Simplify and enhance port Authored by john_saltant.com on Mar 5 2017, 12:34 AM. Tags Referenced Files
Subscribers
Details
In roughly descending order of significance:
Diff Detail
Event TimelineComment Actions I left a few comments inline. I'll remove the obsolete message from the krb5-11[345] ports. It should have been deleted when the ports were split into krb5 and krb5-appl.
Comment Actions Responded to feedback. Eliminate PDF and HTML in favor of DOCS. Discard obsolete pkg-message and README. Comment Actions Incorporated feedback. Responded to questions and open issues.
Comment Actions Looks like Phab doesn't want to let me reply to the last item. Rather than rework your patch to remove the last comment and use pkg-message, use the lastest copy of the port as the base for your patch. Also, I like to keep krb5-113, krb5-114 and krb5-115 in sync. Any changes to krb5-115 will need to be back-ported to krb5-114 and krb5-113. Sometimes we can get away with a single patch that applies to all three. Sometimes not.
Comment Actions I have an example (actually a re-roll of the patch in this ticket). I can upload it if you want or I get it to you some other way if you want. The other thing I didn't notice before was that the proposed patch removes README.FreeBSD entirely (README.FreeBSD.in) should generate that file. Looks like it's not. We'll need to look at that. -share/doc/krb5/README.FreeBSD The following files are also removed and shouldn't be by the patch. They're used as examples of what users should add to kdc.conf, krb5.conf, and services. These have to stay: -share/examples/krb5/kdc.conf Comment Actions Nevermind. I see the change. My example applies DOCS when DOCSPDF and/or DOCSHTML is requested. Comment Actions I'm having some trouble following your comments. It sounds like the thing to do is for me to unbundle this work into a sequence of mostly independent changes so you can review or reject each one in turn, and re-combine them however you think best. That should make my life and yours easier. If you disagree, I'll make another pass at understanding the subthreads and reconciling your comments.
I see you already committed work to the port (r435447-r435449) on this point, so I'll consider it resolved and re-spin my patch(es) accordingly.
I did not remove these files. I converted them to PORTEXAMPLES, which dynamically adds them to the PLIST iff the EXAMPLES option is enabled, which it is by default. I will re-submit this as an independent change. Incidentally, thank you for your prompt responses. I may not get to the next round for a few days now that we're exiting the weekend.
Comment Actions I didn't mean that. We can do one commit for the first seven points. If at a future time the patches need their offsets cleaned up we can do a patch cleanup commit separately. To get an idea of what I'm talking about, rather than pollute this ticket with my patch, check http://cschubert.com/krb5-115.diff. I'd rather the patch come from you than me. I think generally ports committers tend to put too much into a single commit instead of breaking commits into functional pieces. It''s harder to parse out functional diffs that way... When I got my src commit bit my src mentor taught me that each functional commit should be separate from each other, for that very reason. I've taken that wisdom to ports much to the consternation of people who believe that we try to do as much as possible in one commit. But I digress. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||