Re: [Spacewalk-devel] pgsql review
On Thu, May 14, 2009 at 11:24:33AM -0400, Jeff Ortel wrote: The views/rhnHistoryView.sql file seems to still contain definition of rhnHistoryView_pkglist function. Is that correct? Hmm... didn't expect to find function definitions in a view file so I didn't look. I agree this function should be split out and the view.deps updated. This file was just an example. If we are touching the file in any way, even if just moving it from one directory to another, and especially with this large schema restructuralization effort, the commit of the file is basically a seal of correctness. If we did not check the files manually or with some tools, we should not be changing or moving the file. I'm much in favor of schema validation tools which will in rpm build time catch issues like this one. I'm very much against reformatting tools that just change the spacing and lowercase to uppercase, if they do not contain the overall validation parts as well. -- Jan Pazdziora Senior Software Engineer, Satellite Engineering, Red Hat ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Re: Proposed commit hook to catch suspicious merge commits
On Tue, May 12, 2009 at 03:25:53PM +0200, Jan Pazdziora wrote: Jan, is there something I can do to edit that message and get this push to work? The one of d1c65b572d1b9e1ccd863cce3104a853acc9ad9f? Nope. The best bet will probably be to ask the infrastructure guys to disable the hook, do the push, and enable it again. Upon subsequent pushes, the commit will already not be considered by the hook. I was thinking about this more. Can't you merge the commit just before the merge commit which causes a trouble, then cherry pick that commit's change with normal commit, and then merge the HEAD? That way, the git rev-list could be reporting the new non-merge commit, not the merge one. Could you give it a try? -- Jan Pazdziora Senior Software Engineer, Satellite Engineering, Red Hat ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Why rhn-client-tools requires yum-rhn-plugin?
Pradeep Kilambi pkila...@redhat.com wrote: Jan Pazdziora wrote: Aha, so it is not requires, but rather conflicts with older versions. So should be more correct to put inside rhn-client-tools.spec instead of: Requires: yum-rhn-plugin = 0.5.3-30 this line: Conflicts: yum-rhn-plugin 0.5.3-30 ??? I'm not sure about yum-rhn-plugin.spec since I did not check its code and dunno if there should be conflict as well or if the code is really required. Its not a conflict. We should force users using latest yum-rhnplugin to get newer rhn-client-tools and vice-versa. So requires as we have should do it. Prad, if the user does have yum-rhnplugin but not rhn-client-tools, will yum-rhnplugin work? If it won't then it's Requires case. If it will work without rhn-client-tools or with new rhn-client-tools but not with old rhn-client-tools, it's Conflicts. And vice versa -- for does rhn-client-tools need yum-rhnplugin to work? yes, without yum-rhn-plugin, rhn_check(part of rhn-client-tools) will not be able to process actions picked up and pass it to yum. Similarly without rhn-client-tools, yum-rhn-plugin will not be able to authenticate to rhn, as rhn-client-tools is what provides the authentication layer. There are also other cases where, the versioning is enforced in the http headers through clientCaps in rhn-client-tools to get the access to various capabilities such as arch info from the supported server. But this arch info can only be processed by the latest yum-rhn-plugin which has the arch capability fixes and which depends on the new rebased yum in 5.3. And hence a Requires case in my opinion. Hmm, then yum-rhn-plugin should be required by rhn_check, which is not. It is currently required by rhn-client-tools and rhn-custom-info. It seems to me that rhn-client-tools is only library and should not require anything else (well I speak about yum-rhn-plugin). Other subpackages from rhn-client-tools requires yum-rhn-plugin and it is fine, but I did not reason for the main rhn-client-tools. Mirek ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Why rhn-client-tools requires yum-rhn-plugin?
- Pradeep Kilambi pkila...@redhat.com wrote: Miroslav Suchy wrote: Aha, so it is not requires, but rather conflicts with older versions. So should be more correct to put inside rhn-client-tools.spec instead of: Requires: yum-rhn-plugin = 0.5.3-30 this line: Conflicts: yum-rhn-plugin 0.5.3-30 ??? Its not a conflict. We should force users using latest yum-rhnplugin to get newer rhn-client-tools and vice-versa. So requires as we have should do it. ??? If he do yum upgrade, he will get both in latest version. If he do yum install yum-rhn-plugin he will get latest rhn-client-tools since yum-rhn-plugin: rhn-client-tools = 0.4.19-9 And if he fore some reason will do: rpm -Uvh rhn-client-tools-0.4.25-1.rpm with some old yum-rhn-plugin, then he will get warning that this version conflict with yum-rhn-plugin 0.5.3-30, so he will be forced to do rpm -Uvh rhn-client-tools-0.4.25-1.rpm yum-rhn-plugin-0.5.3-30.rpm But again, if there will be conflict and user use only yum and will not try to install package one by one, he will notice nothing. ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] pgsql review
Jeff Ortel wrote: % Jan, I appreciate you comments :) % % Jan Pazdziora wrote: % On Thu, May 14, 2009 at 11:24:33AM -0400, Jeff Ortel wrote: % The views/rhnHistoryView.sql file seems to still contain definition of % rhnHistoryView_pkglist function. Is that correct? % Hmm... didn't expect to find function definitions in a view file so I % didn't look. I agree this function should be split out and the view.deps % updated. % % This file was just an example. % % If we are touching the file in any way, even if just moving it from % one directory to another, and especially with this large schema % restructuralization effort, the commit of the file is basically a seal % of correctness. If we did not check the files manually or with some % tools, we should not be changing or moving the file. % % This really isn't practical. This was a massive effort. We had to touch % 400+ files and certifying semantic correctness on files we simply moved is % asking a lot. The goal was semantic equivalences which can be checked by % validating the installed schema. Well, but once the 'constraint wust_type_nn not null' was changed to 'NOT NULL' the schemas will not be equivalent. Contraint names will differ. Although it doesn't matter for satellite operation it's unacceptable change for the upgrades. % I'm much in favor of schema validation tools which will in rpm % build time catch issues like this one. % I'm very much against % reformatting tools that just change the spacing and lowercase to % uppercase, if they do not contain the overall validation parts as well. % % % Chameleon is not a formatting tool and was not applied to simply change % spacing and case. It was used to automate some of the refactoring process % and the reformatting was a byproduct. Besides, the files examples you've % sited for semantic checking were not reformatted at all. Seems like your % primary disagreement is that the files were reformatted at all which makes % them difficult to diff. If everyone feels that reformatting these files % prevents accepting this work into master, we can re-implement this part of % the refactoring. One issue per commit, please. Chemeleon did refactoring + reformatting + code optimization. How can anyone review the real changes? I've got completely lost in the diffs... :( -- Michael Mráka Satellite Engineering, Red Hat ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] pgsql review
Michael Mraka wrote: Jeff Ortel wrote: % Jan, I appreciate you comments :) % % Jan Pazdziora wrote: % On Thu, May 14, 2009 at 11:24:33AM -0400, Jeff Ortel wrote: % The views/rhnHistoryView.sql file seems to still contain definition of % rhnHistoryView_pkglist function. Is that correct? % Hmm... didn't expect to find function definitions in a view file so I % didn't look. I agree this function should be split out and the view.deps % updated. % % This file was just an example. % % If we are touching the file in any way, even if just moving it from % one directory to another, and especially with this large schema % restructuralization effort, the commit of the file is basically a seal % of correctness. If we did not check the files manually or with some % tools, we should not be changing or moving the file. % % This really isn't practical. This was a massive effort. We had to touch % 400+ files and certifying semantic correctness on files we simply moved is % asking a lot. The goal was semantic equivalences which can be checked by % validating the installed schema. Well, but once the 'constraint wust_type_nn not null' was changed to 'NOT NULL' the schemas will not be equivalent. Contraint names will differ. Although it doesn't matter for satellite operation it's unacceptable change for the upgrades. I checked this before making the change. ALTER TABLE A modify c1 null; This command will remove the NOT NULL constraint regardless of whether it was created by adding a named constraint of using the NOT NULL keywords. % I'm much in favor of schema validation tools which will in rpm % build time catch issues like this one. % I'm very much against % reformatting tools that just change the spacing and lowercase to % uppercase, if they do not contain the overall validation parts as well. % % % Chameleon is not a formatting tool and was not applied to simply change % spacing and case. It was used to automate some of the refactoring process % and the reformatting was a byproduct. Besides, the files examples you've % sited for semantic checking were not reformatted at all. Seems like your % primary disagreement is that the files were reformatted at all which makes % them difficult to diff. If everyone feels that reformatting these files % prevents accepting this work into master, we can re-implement this part of % the refactoring. One issue per commit, please. Chemeleon did refactoring + reformatting + code optimization. How can anyone review the real changes? I've got completely lost in the diffs... :( -- Michael Mráka Satellite Engineering, Red Hat ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
