Derby
  1. Derby
  2. DERBY-1400

Cleanup code in network server's DRDAStatement class

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 10.1.3.1, 10.2.1.6
    • Fix Version/s: 10.9.2.2, 10.10.1.1
    • Component/s: Network Server
    • Labels:
      None
    • Issue & fix info:
      Newcomer

      Description

      The following cleanup tasks were identified as part of DERBY-1002 (http://issues.apache.org/jira/browse/DERBY-1002):

      • pkgcnstkn, pkgid, pkgsn variables can be removed from DRDAStatement since these are derived from pkgnamcsn object.
      • Look into what is required by initialize() of default statement. Currently, initialize just calls setTypDefValues(). Once the purpose of this method is confirmed, we may need to modify the comments at places it is currently called.
      1. derby-1400_diff.txt
        5 kB
        Kathey Marsden
      2. DERBY-1400.diff
        33 kB
        Abhilash T.G

        Activity

        Gavin made changes -
        Workflow jira [ 12373770 ] Default workflow, editable Closed status [ 12801762 ]
        Kathey Marsden made changes -
        Fix Version/s 10.9.2.0 [ 12323562 ]
        Fix Version/s 10.9.1.1 [ 12321551 ]
        Kathey Marsden made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 10.9.1.1 [ 12321551 ]
        Fix Version/s 10.10.0.0 [ 12321550 ]
        Resolution Fixed [ 1 ]
        Hide
        Kathey Marsden added a comment -

        Resolving this issue fixed in trunk
        http://svn.apache.org/viewvc?view=revision&revision=1371677
        and 10.9
        http://svn.apache.org/viewvc?view=revision&revision=1373137

        despite my earlier comments I won't take it back to 10.8 right now

        Show
        Kathey Marsden added a comment - Resolving this issue fixed in trunk http://svn.apache.org/viewvc?view=revision&revision=1371677 and 10.9 http://svn.apache.org/viewvc?view=revision&revision=1373137 despite my earlier comments I won't take it back to 10.8 right now
        Hide
        Kathey Marsden added a comment -

        so svn merge with -x -b seems to work great after the reformat of DERBY-5896. Merging to 10.9 with the command below merges cleanly. The new lines of course have spaces vs the old tab format but that is typical practice for a lot of changes anyway.

        $svn merge -x -b -c 1371677 https://svn.apache.org/repos/asf/db/derby/code/trunk

        Show
        Kathey Marsden added a comment - so svn merge with -x -b seems to work great after the reformat of DERBY-5896 . Merging to 10.9 with the command below merges cleanly. The new lines of course have spaces vs the old tab format but that is typical practice for a lot of changes anyway. $svn merge -x -b -c 1371677 https://svn.apache.org/repos/asf/db/derby/code/trunk
        Hide
        Myrna van Lunteren added a comment -

        I did a visual check of the patch, and it seems to do the same as before, while removing the variables. +1 to commit (assuming the tests run ok).

        Show
        Myrna van Lunteren added a comment - I did a visual check of the patch, and it seems to do the same as before, while removing the variables. +1 to commit (assuming the tests run ok).
        Kathey Marsden made changes -
        Attachment derby-1400_diff.txt [ 12540076 ]
        Hide
        Kathey Marsden added a comment -

        Attached is a patch for this issue making the changes Deepa suggests. I have only run derbynet._Suite so far. Running full regression tests now.

        This patch is just code cleanup but I plan to backport to 10.8 as a test of post reformat merges.

        Show
        Kathey Marsden added a comment - Attached is a patch for this issue making the changes Deepa suggests. I have only run derbynet._Suite so far. Running full regression tests now. This patch is just code cleanup but I plan to backport to 10.8 as a test of post reformat merges.
        Kathey Marsden made changes -
        Assignee Kathey Marsden [ kmarsden ]
        Dag H. Wanvik made changes -
        Issue & fix info [Newcomer]
        Dag H. Wanvik made changes -
        Component/s Newcomer [ 12310640 ]
        Dag H. Wanvik made changes -
        Derby Categories [Newcomer]
        Kathey Marsden made changes -
        Assignee Abhilash T.G [ abhi-1 ]
        Hide
        Kathey Marsden added a comment -

        Unassigning due to inactivity. Please reassign yourself if you would like to work on this issue.

        Show
        Kathey Marsden added a comment - Unassigning due to inactivity. Please reassign yourself if you would like to work on this issue.
        Dyre Tjeldvoll made changes -
        Derby Info [Patch Available]
        Hide
        Dyre Tjeldvoll added a comment -

        Let's see if I can manage to uncheck the box this time...

        Show
        Dyre Tjeldvoll added a comment - Let's see if I can manage to uncheck the box this time...
        Hide
        Dyre Tjeldvoll added a comment -

        Removing patch flag since the patch can no longer be applied.

        Show
        Dyre Tjeldvoll added a comment - Removing patch flag since the patch can no longer be applied.
        Hide
        Kristian Waagan added a comment -

        Not quite sure what you are asking about, but I'll try to help.

        Regarding the patch, remove all changes that are not directly related to your cleanup of the DRDAStatement code, like adding/removing/replacing spaces or tabs at places where you haven't done any other change.

        You should also run the tests to make sure you haven't broken anything. You can find instructions and information at the wiki, and I think all the relevant pointers are in the following two mail threads:
        http://www.nabble.com/Derby-Testing-td15205194.html
        http://www.nabble.com/Newbie-to-Derby-Testing-td15312500.html

        I (or someone else ) will have another look at the updated patch after the unnecessary whitespace changes have been removed.

        I know running the tests can be a bit of a pain the first times, but it is something you have to go through if you plan to contribute patches.

        BTW: I noticed that the patch does not apply to trunk any more due to a conflicting change in the same file. It also conflicts if I revert to the revision the patch was generated from and then do svn up. So I think you might have to generate a new patch.

        Show
        Kristian Waagan added a comment - Not quite sure what you are asking about, but I'll try to help. Regarding the patch, remove all changes that are not directly related to your cleanup of the DRDAStatement code, like adding/removing/replacing spaces or tabs at places where you haven't done any other change. You should also run the tests to make sure you haven't broken anything. You can find instructions and information at the wiki, and I think all the relevant pointers are in the following two mail threads: http://www.nabble.com/Derby-Testing-td15205194.html http://www.nabble.com/Newbie-to-Derby-Testing-td15312500.html I (or someone else ) will have another look at the updated patch after the unnecessary whitespace changes have been removed. I know running the tests can be a bit of a pain the first times, but it is something you have to go through if you plan to contribute patches. BTW: I noticed that the patch does not apply to trunk any more due to a conflicting change in the same file. It also conflicts if I revert to the revision the patch was generated from and then do svn up. So I think you might have to generate a new patch.
        Hide
        Abhilash T.G added a comment -

        i dont know much about the standards..can you pls elaborate on that.

        Show
        Abhilash T.G added a comment - i dont know much about the standards..can you pls elaborate on that.
        Hide
        Abhilash T.G added a comment -

        ca you pls help me with it.

        Show
        Abhilash T.G added a comment - ca you pls help me with it.
        Hide
        Kristian Waagan added a comment -

        Thanks for providing the patch, Abhilash.

        I had a quick look at it, and it seems to me it is a lot bigger than it has to be because of unrelated whitespace changes.
        There is a preference in the Derby community to leave unrelated whitespace changes out of patches fixing other issues. I do notice that the class you are working on (DRDAStatement) contains a mix of tabs and spaces, and also a number of lines with trailing whitespace.

        The current policy for new Derby source code is to use four spaces for indentation.

        1) Can you please make another patch, where you don't incorporate unrelated whitespace changes?

        2) Have you run any regression tests for the changes?

        If you have questions, feel free to ask!

        thanks,

        Show
        Kristian Waagan added a comment - Thanks for providing the patch, Abhilash. I had a quick look at it, and it seems to me it is a lot bigger than it has to be because of unrelated whitespace changes. There is a preference in the Derby community to leave unrelated whitespace changes out of patches fixing other issues. I do notice that the class you are working on (DRDAStatement) contains a mix of tabs and spaces, and also a number of lines with trailing whitespace. The current policy for new Derby source code is to use four spaces for indentation. 1) Can you please make another patch, where you don't incorporate unrelated whitespace changes? 2) Have you run any regression tests for the changes? If you have questions, feel free to ask! thanks,
        Myrna van Lunteren made changes -
        Derby Info [Patch Available]
        Abhilash T.G made changes -
        Attachment DERBY-1400.diff [ 12374355 ]
        Hide
        Abhilash T.G added a comment -

        made tha changes

        Show
        Abhilash T.G added a comment - made tha changes
        Abhilash T.G made changes -
        Field Original Value New Value
        Assignee Abhilash T.G [ abhi-1 ]
        Hide
        Abhilash T.G added a comment -

        I would like to work on this

        Show
        Abhilash T.G added a comment - I would like to work on this
        Deepa Remesh created issue -

          People

          • Assignee:
            Kathey Marsden
            Reporter:
            Deepa Remesh
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development