[PATCH] 0015-Removal-of-unused-private-methods
I'm struggling with the premise behind this patch. Some of these methods seem like they are valuable - and certainly they were valuable at some point in time (or they would not have been written). On the other hand, I see the benefit of trimming the code base. No need to refactor code that isn't being used. I realize that this removes some eclipse warnings, but is removing these methods good practice? I guess I'm looking for some other folks to chime in here. Ade On Thu, 2011-11-10 at 22:29 -0500, Adam Young wrote:
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
On 11/16/2011 09:48 AM, Ade Lee wrote:
I'm struggling with the premise behind this patch. Some of these methods seem like they are valuable - and certainly they were valuable at some point in time (or they would not have been written). On the other hand, I see the benefit of trimming the code base. No need to refactor code that isn't being used.
I realize that this removes some eclipse warnings, but is removing these methods good practice?
I guess I'm looking for some other folks to chime in here.
My feeling is that you do not leave dead code in the code base. The code still exists in the repository, but it is unlikely that a piece of code that is not called today will be needed in exactly the same form in the future. Code is documentation. Code that is not used is misleading to the maintainer. Committing commented out code is a bad practice. As we refactor, the location of the behavior of a lot of these methods will change. Thus, some orphan calls will become uncallable, and others will get in the way of code clean up. This is old code: some of it goes back 15 years. Holding on to unused, uncallable code is akin to hoarding.
Ade
On Thu, 2011-11-10 at 22:29 -0500, Adam Young wrote:
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
On 11/16/2011 10:38 AM, Adam Young wrote:
On 11/16/2011 09:48 AM, Ade Lee wrote:
I'm struggling with the premise behind this patch. Some of these methods seem like they are valuable - and certainly they were valuable at some point in time (or they would not have been written). On the other hand, I see the benefit of trimming the code base. No need to refactor code that isn't being used.
I realize that this removes some eclipse warnings, but is removing these methods good practice?
I guess I'm looking for some other folks to chime in here.
My feeling is that you do not leave dead code in the code base. The code still exists in the repository, but it is unlikely that a piece of code that is not called today will be needed in exactly the same form in the future.
Code is documentation. Code that is not used is misleading to the maintainer. Committing commented out code is a bad practice.
As we refactor, the location of the behavior of a lot of these methods will change. Thus, some orphan calls will become uncallable, and others will get in the way of code clean up.
This is old code: some of it goes back 15 years. Holding on to unused, uncallable code is akin to hoarding.
Ade
On Thu, 2011-11-10 at 22:29 -0500, Adam Young wrote:
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
Conflicted with the PKI Silent changes, so patch has been remade by hand.
On 12/07/2011 11:07 AM, Adam Young wrote:
On 11/16/2011 10:38 AM, Adam Young wrote:
On 11/16/2011 09:48 AM, Ade Lee wrote:
I'm struggling with the premise behind this patch. Some of these methods seem like they are valuable - and certainly they were valuable at some point in time (or they would not have been written). On the other hand, I see the benefit of trimming the code base. No need to refactor code that isn't being used.
I realize that this removes some eclipse warnings, but is removing these methods good practice?
I guess I'm looking for some other folks to chime in here.
My feeling is that you do not leave dead code in the code base. The code still exists in the repository, but it is unlikely that a piece of code that is not called today will be needed in exactly the same form in the future.
Code is documentation. Code that is not used is misleading to the maintainer. Committing commented out code is a bad practice.
As we refactor, the location of the behavior of a lot of these methods will change. Thus, some orphan calls will become uncallable, and others will get in the way of code clean up.
This is old code: some of it goes back 15 years. Holding on to unused, uncallable code is akin to hoarding.
Ade
On Thu, 2011-11-10 at 22:29 -0500, Adam Young wrote:
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
Conflicted with the PKI Silent changes, so patch has been remade by hand.
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel Withdrawn., The formatting changes make it impractical to try and reformat this. Will be resubmitted in smaller patches shortly.
On 12/08/2011 03:00 PM, Adam Young wrote:
On 12/07/2011 11:07 AM, Adam Young wrote:
On 11/16/2011 10:38 AM, Adam Young wrote:
On 11/16/2011 09:48 AM, Ade Lee wrote:
I'm struggling with the premise behind this patch. Some of these methods seem like they are valuable - and certainly they were valuable at some point in time (or they would not have been written). On the other hand, I see the benefit of trimming the code base. No need to refactor code that isn't being used.
I realize that this removes some eclipse warnings, but is removing these methods good practice?
I guess I'm looking for some other folks to chime in here.
My feeling is that you do not leave dead code in the code base. The code still exists in the repository, but it is unlikely that a piece of code that is not called today will be needed in exactly the same form in the future.
Code is documentation. Code that is not used is misleading to the maintainer. Committing commented out code is a bad practice.
As we refactor, the location of the behavior of a lot of these methods will change. Thus, some orphan calls will become uncallable, and others will get in the way of code clean up.
This is old code: some of it goes back 15 years. Holding on to unused, uncallable code is akin to hoarding.
Ade
On Thu, 2011-11-10 at 22:29 -0500, Adam Young wrote:
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
Conflicted with the PKI Silent changes, so patch has been remade by hand.
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel Withdrawn., The formatting changes make it impractical to try and reformat this. Will be resubmitted in smaller patches shortly.
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel Rebased on top of the format changes
On 12/08/2011 03:00 PM, Adam Young wrote:
On 12/07/2011 11:07 AM, Adam Young wrote:
On 11/16/2011 10:38 AM, Adam Young wrote:
On 11/16/2011 09:48 AM, Ade Lee wrote:
I'm struggling with the premise behind this patch. Some of these methods seem like they are valuable - and certainly they were valuable at some point in time (or they would not have been written). On the other hand, I see the benefit of trimming the code base. No need to refactor code that isn't being used.
I realize that this removes some eclipse warnings, but is removing these methods good practice?
I guess I'm looking for some other folks to chime in here.
My feeling is that you do not leave dead code in the code base. The code still exists in the repository, but it is unlikely that a piece of code that is not called today will be needed in exactly the same form in the future.
Code is documentation. Code that is not used is misleading to the maintainer. Committing commented out code is a bad practice.
As we refactor, the location of the behavior of a lot of these methods will change. Thus, some orphan calls will become uncallable, and others will get in the way of code clean up.
This is old code: some of it goes back 15 years. Holding on to unused, uncallable code is akin to hoarding.
Ade
On Thu, 2011-11-10 at 22:29 -0500, Adam Young wrote:
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com <mailto:Pki-devel@redhat.com> https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com <mailto:Pki-devel@redhat.com> https://www.redhat.com/mailman/listinfo/pki-devel
Conflicted with the PKI Silent changes, so patch has been remade by hand.
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com <mailto:Pki-devel@redhat.com> https://www.redhat.com/mailman/listinfo/pki-devel Withdrawn., The formatting changes make it impractical to try and reformat this. Will be resubmitted in smaller patches shortly.
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com <mailto:Pki-devel@redhat.com> https://www.redhat.com/mailman/listinfo/pki-devel Rebased on top of the format changes
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel ACK - although the "reformatting" may need to be changed since this
On 12/08/11 18:25, Adam Young wrote: patch was based upon having the formatting patch applied which has since been reverted. After performing the following commands, I also was able to successfully apply this patch using the following commands: * git reset --hard HEAD~1 (from "master" --- was used to remove a previously broken patch) * git branch unused_private_methods * git checkout unused_private_methods * git am --whitespace=fix --signoff < dogtag-admiyo-0015-2-Removal-of-unused-private-methods.patch I was able to successfully build this branch from both Eclipse and using the "pki/scripts/compose_pki_core_packages" script. I installed the packages that I built, ran "pkicreate", configured the CA via the Firefox browser, and successfully enrolled a certificate. -- Matt
On 12/20/2011 05:36 PM, Matthew Harmsen wrote:
On 12/08/2011 03:00 PM, Adam Young wrote:
On 12/07/2011 11:07 AM, Adam Young wrote:
On 11/16/2011 10:38 AM, Adam Young wrote:
On 11/16/2011 09:48 AM, Ade Lee wrote:
I'm struggling with the premise behind this patch. Some of these methods seem like they are valuable - and certainly they were valuable at some point in time (or they would not have been written). On the other hand, I see the benefit of trimming the code base. No need to refactor code that isn't being used.
I realize that this removes some eclipse warnings, but is removing these methods good practice?
I guess I'm looking for some other folks to chime in here.
My feeling is that you do not leave dead code in the code base. The code still exists in the repository, but it is unlikely that a piece of code that is not called today will be needed in exactly the same form in the future.
Code is documentation. Code that is not used is misleading to the maintainer. Committing commented out code is a bad practice.
As we refactor, the location of the behavior of a lot of these methods will change. Thus, some orphan calls will become uncallable, and others will get in the way of code clean up.
This is old code: some of it goes back 15 years. Holding on to unused, uncallable code is akin to hoarding.
Ade
On Thu, 2011-11-10 at 22:29 -0500, Adam Young wrote: > _______________________________________________ > Pki-devel mailing list > Pki-devel@redhat.com <mailto:Pki-devel@redhat.com> > https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com <mailto:Pki-devel@redhat.com> https://www.redhat.com/mailman/listinfo/pki-devel
Conflicted with the PKI Silent changes, so patch has been remade by hand.
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com <mailto:Pki-devel@redhat.com> https://www.redhat.com/mailman/listinfo/pki-devel Withdrawn., The formatting changes make it impractical to try and reformat this. Will be resubmitted in smaller patches shortly.
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com <mailto:Pki-devel@redhat.com> https://www.redhat.com/mailman/listinfo/pki-devel Rebased on top of the format changes
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel ACK - although the "reformatting" may need to be changed since this
On 12/08/11 18:25, Adam Young wrote: patch was based upon having the formatting patch applied which has since been reverted.
After performing the following commands, I also was able to successfully apply this patch using the following commands:
* git reset --hard HEAD~1 (from "master" --- was used to remove a previously broken patch) * git branch unused_private_methods * git checkout unused_private_methods * git am --whitespace=fix --signoff < dogtag-admiyo-0015-2-Removal-of-unused-private-methods.patch
I was able to successfully build this branch from both Eclipse and using the "pki/scripts/compose_pki_core_packages" script.
I installed the packages that I built, ran "pkicreate", configured the CA via the Firefox browser, and successfully enrolled a certificate.
-- Matt
Pushed to master
participants (3)
-
Adam Young -
Ade Lee -
Matthew Harmsen