[kepler-dev] @author tags in duplicated code.
Christopher Brooks
cxh at eecs.berkeley.edu
Wed Oct 20 16:08:27 PDT 2010
Hi Bertram,
My comments are below.
On 10/20/10 2:50 PM, Bertram Ludaescher wrote:
> Hi Christopher:
>
> Re. attribution: I agree -- the @author tag needs to reflect who the authors of a file actually were.
> I assume there are some conventions how that's done, e.g. in chronological order.
> So I guess when experimenting with existing code, one would list the original authors plus the experimenter, right?
Right. Use
@author Your Name, Based on XXX.java by Steve Neuendorffer and Edward A. Lee, Elaine Cheong
Or
@author Your Name, Contributors: Steve Neuendorffer and Edward A. Lee, Elaine Cheong
There is an open bug that involves fixing these in the Kepler release, see
http://bugzilla.ecoinformatics.org/show_bug.cgi?id=4926
>
> Re. duplication: As far as I can tell, Sven is experimenting with some GUI changes that we're working on for the COMAD MoC (replacing actors via drag-and-drop, inserting new actors by dropping an
> actor on a wire).
>
> What is the recommended way to experiment with those GUI changes?
> Making the changes in the "production code" doesn't seem to be an option.
> So is branching or code-duplication the way to go?
My opinion is that code duplication is a hack that usually results in more work.
What I would do is figure out what is necessary to modify the base class and go from there.
To sum up, if people want to do what I consider to be a waste of their time and check in
duplicate code, then I can't really stop it, especially if it off in a module that is not
part of the default Kepler build. However, it is required that they adjust the @author tags.
Edward feels rather strongly about this, as do I. Basically, I don't want to be given credit
for code duplication :-)
Working with some test code to determine what sort of extensions or modifications need to be made
to the base class seems reasonable. I don't think that checking in duplicate classes would be
considered good software engineering.
_Christopher
> I think there needs to be an easy way to make code changes, extensions, etc in an "experimentation area" of the repository. Sometimes it will make sense to fold those changes back into the general code.
> In this particular case, I'm not sure. For example replacing one COMAD actor with another one is easy since the single input, single output structure of actors, together with scope parameters makes
> this replacement meaningful / conceptually easy. This might not be the case for general models / workflows.
>
> Bertram
>
> On Wed, Oct 20, 2010 at 2:13 PM, Christopher Brooks <cxh at eecs.berkeley.edu <mailto:cxh at eecs.berkeley.edu>> wrote:
>
> [I'm changing this from kepler-code to kepler-dev]
>
> Hi Bertram,
> I can guess what Sven was trying to do, but I certainly don't know for sure.
>
> There are two issues: proper attribution and code duplication.
>
> The problem is that Edward did not edit that file, and feels rather strongly that
> his name should not appear on code that he has not edited. The right thing when
> basing a class on other source code is to give credit for the author using the
> @author tag below.
>
> The code duplication issue is slightly different issue. I'm not sure what Sven's
> intentions are here. In the past, we have spent many hours cleaning up duplicate code.
> It is better to see about modifying the parent classes.
>
> There were some other classes that could be duplicates:
>
>
> Log:
> initial version of GUI modifications including:
> - drag and drop replacement of actors with reconecting ports and copying parameters
> - don't layout hidden ports that are not connected (helpful for COMAD)
>
> trunk/modules/gui-mod/src/ptolemy/vergil/actor/ActorController.java
> trunk/modules/gui-mod/src/ptolemy/vergil/basic/
> trunk/modules/gui-mod/src/ptolemy/vergil/basic/EditorDropTarget.java
>
>
> Log:
> moving gui modifications to their own module
>
> trunk/modules/sven_CPES/src/ptolemy/vergil/actor/ActorController.java
>
>
> If the base classes need different extension points, then I'd gladly accept
> well documented changes that following the Ptolemy style guide.
>
> _Christopher
>
>
>
>
>
> On 10/20/10 1:53 PM, Bertram Ludaescher wrote:
>
> Hi Christopher:
>
> Do you know what Sven was trying to do and what would be the correct way to proceed for him in this case?
>
> Thanks
>
> Bertram
>
> On Tue, Oct 19, 2010 at 8:22 PM, Christopher Brooks <cxh at eecs.berkeley.edu <mailto:cxh at eecs.berkeley.edu> <mailto:cxh at eecs.berkeley.edu <mailto:cxh at eecs.berkeley.edu>>> wrote:
>
> Hi,
> In general code duplication is a bad thing.
>
> If you are going to duplicate ptolemy classes, then
> please change the @author tags so that that
> Edward and the others are not listed as authors. Instead of
> @author Steve Neuendorffer and Edward A. Lee, Elaine Cheong
> Try:
> @author Your Name, Based on XXX.java by Steve Neuendorffer and Edward A. Lee, Elaine Cheong
>
> _Christopher
>
> On 10/19/10 11:59 AM, koehler at ecoinformatics.org <mailto:koehler at ecoinformatics.org> <mailto:koehler at ecoinformatics.org <mailto:koehler at ecoinformatics.org>> wrote:
>
> Author: koehler
> Date: 2010-10-19 11:59:16 -0700 (Tue, 19 Oct 2010)
> New Revision: 26115
>
> Added:
> trunk/modules/gui-mod/
> trunk/modules/gui-mod/lib/
> trunk/modules/gui-mod/lib/exe/
> trunk/modules/gui-mod/resources/
> trunk/modules/gui-mod/resources/system.properties/
> trunk/modules/gui-mod/src/
> trunk/modules/gui-mod/src/ptolemy/
> trunk/modules/gui-mod/src/ptolemy/vergil/
> trunk/modules/gui-mod/src/ptolemy/vergil/actor/
> trunk/modules/gui-mod/src/ptolemy/vergil/actor/ActorController.java
> trunk/modules/gui-mod/src/ptolemy/vergil/basic/
> trunk/modules/gui-mod/src/ptolemy/vergil/basic/EditorDropTarget.java
> Log:
> initial version of GUI modifications including:
> - drag and drop replacement of actors with reconecting ports and copying parameters
> - don't layout hidden ports that are not connected (helpful for COMAD)
>
>
> Added: trunk/modules/gui-mod/src/ptolemy/vergil/actor/ActorController.java
>
> ...
>
> + * @author Steve Neuendorffer and Edward A. Lee, Elaine Cheong
> + * @version $Id: ActorController.java 57046 2010-01-27 23:35:53Z cxh $
> + * @since Ptolemy II 2.0
> + * @Pt.ProposedRating Red (eal)
> + * @Pt.AcceptedRating Red (johnr)
> + * @see ActorInstanceController
> + * @see ClassDefinitionController
> +*/
> +public abstract class ActorController extends AttributeController {
>
> ...
>
> _______________________________________________
> Kepler-cvs mailing list
> Kepler-cvs at kepler-project.org <mailto:Kepler-cvs at kepler-project.org> <mailto:Kepler-cvs at kepler-project.org <mailto:Kepler-cvs at kepler-project.org>>
>
> http://lists.nceas.ucsb.edu/kepler/mailman/listinfo/kepler-cvs
>
>
>
> --
> Christopher Brooks, PMP University of California
> CHESS Executive Director US Mail: 337 Cory Hall
> Programmer/Analyst CHESS/Ptolemy/Trust Berkeley, CA 94720-1774
> ph: 510.643.9841 fax:510.642.2718 (Office: 545Q Cory)
> home: (F-Tu) 707.665.0131 cell: 707.332.0670
>
>
--
Christopher Brooks, PMP University of California
CHESS Executive Director US Mail: 337 Cory Hall
Programmer/Analyst CHESS/Ptolemy/Trust Berkeley, CA 94720-1774
ph: 510.643.9841 fax:510.642.2718 (Office: 545Q Cory)
home: (F-Tu) 707.665.0131 cell: 707.332.0670
More information about the Kepler-dev
mailing list