From: James R. <sj...@jd...> - 2005-11-27 01:35:22
Attachments:
author-page-real-name.patch
|
Hi all, This patch makes the author page display real names (if available). You can see an example of the output generated by this patch at http://xtuml.jdns.org/statcvs/user_u3957053.html Please note that only the page title is changed. No other output is affected. I'll also post this on the SourceForge patch tracker. Regards, James -- James Ring |
From: Richard C. <ri...@cy...> - 2005-11-30 19:46:36
|
Hi James, Thanks for the patches. I think that adding external author metadata (real names, but also email addresses and a link to a web page) is a useful addition. But I don't like the way you've architected it. Other users may want to get author data from other sources than passwd files, and your patch would make it a bit hard to add other sources. I'd suggest you change the patch to keep the passwd-specific logic out of the core classes. Some ideas for this: - Store the real names not in the Author instances, but in a new class, say PasswdFile, with a getRealName(Author) method. - make PasswdFile an implementation of a new interface, say AuthorProfilesIntegration, which has just that method for the moment. This is the same general approach as with File and WebRepositoryIntegration. Please have a look at how these classes are used for inspiration. Note that we have a couple of different WebRepositoryIntegration implementations, and adding new ones is easy. I'd like to have the same possiblity for external author data. Steffen, any insight on this? What would be a good way to support both Maven and passwd files as data sources? Cheers, Richard On 27 Nov 2005, at 02:35, James Ring wrote: > Hi all, > > This patch makes the author page display real names (if available). > You can > see an example of the output generated by this patch at > > http://xtuml.jdns.org/statcvs/user_u3957053.html > > Please note that only the page title is changed. No other output is > affected. > > I'll also post this on the SourceForge patch tracker. > > Regards, > James > -- > James Ring > <author-page-real-name.patch> |
From: James R. <sj...@jd...> - 2005-11-30 21:40:39
|
Hi Richard, On Thursday 01 December 2005 06:46, Richard Cyganiak wrote: > Hi James, > > Thanks for the patches. I think that adding external author metadata > (real names, but also email addresses and a link to a web page) is a > useful addition. But I don't like the way you've architected it. > Other users may want to get author data from other sources than > passwd files, and your patch would make it a bit hard to add other > sources. > > I'd suggest you change the patch to keep the passwd-specific logic > out of the core classes. Some ideas for this: > > - Store the real names not in the Author instances, but in a new > class, say PasswdFile, with a getRealName(Author) method. > - make PasswdFile an implementation of a new interface, say > AuthorProfilesIntegration, which has just that method for the moment. Ok, I think I agree in principle, but I don't like the name=20 (AuthorProfilesIntegration). How about just a superclass AuthorProfile, and= =20 subclasses like MavenAuthorProfile or PasswdAuthorProfile or=20 LDAPAuthorProfile ... ? I still think the method getRealName() should remain on the Author class.=20 After all, Authors *do* have real names. Maybe it could just call into the= =20 author's profile. I don't like the idea of having to say AuthorProfilesIntegration i =3D getProfileIntegration(); /* or whatever */ i.getRealName( author ); I think that's backwards. ;) Better to say =09 Author a =3D /* ... */ a.getRealName(); // or a.getProfile().getRealName(); or something like that. > This is the same general approach as with File and > WebRepositoryIntegration. Please have a look at how these classes are > used for inspiration. Note that we have a couple of different > WebRepositoryIntegration implementations, and adding new ones is > easy. I'd like to have the same possiblity for external author data. That's fair enough, and I think it's possible with an AuthorProfile, but I'= ll=20 send patches and see what you think. > Steffen, any insight on this? What would be a good way to support > both Maven and passwd files as data sources? > > Cheers, > Richard > Thanks for your time. Regards, James =2D-=20 James Ring |
From: Richard C. <ri...@cy...> - 2005-11-30 22:26:49
|
On 30 Nov 2005, at 22:40, James Ring wrote: > Ok, I think I agree in principle, but I don't like the name > (AuthorProfilesIntegration). How about just a superclass > AuthorProfile, and > subclasses like MavenAuthorProfile or PasswdAuthorProfile or > LDAPAuthorProfile ... ? Yeah, the name sucks. I think the class should not represent a single profile, but a method of getting author profiles. It's clear what the implementation classes would be, things like PasswdFile, MavenProjectFile, SourceforgeWebsiteScraper. Now find a good common name for these. AuthorProfileSource? > I still think the method getRealName() should remain on the Author > class. > After all, Authors *do* have real names. Maybe it could just call > into the > author's profile. Imagine that profiles may provide other things except real names. Email address, web link etc. There's no need to duplicate all these methods on both the Author and Profiles class. Also, I see no need why the Author class should know about the existence of the profiles. I want the Author class to remain as close as possible to the concept of a CVS login. We're discussing a presentation issue, and changing stuff on the presentation layer shouldn't affect the core CVS classes. (Actually, the Author class should probably be called CVSLogin. My suggestions may make more sense if seen from that point of view.) Richard > I don't like the idea of having to say > > AuthorProfilesIntegration i = getProfileIntegration(); /* or > whatever */ > i.getRealName( author ); > > I think that's backwards. ;) > > Better to say > > Author a = /* ... */ > a.getRealName(); > // or > a.getProfile().getRealName(); > > or something like that. > >> This is the same general approach as with File and >> WebRepositoryIntegration. Please have a look at how these classes are >> used for inspiration. Note that we have a couple of different >> WebRepositoryIntegration implementations, and adding new ones is >> easy. I'd like to have the same possiblity for external author data. > > That's fair enough, and I think it's possible with an > AuthorProfile, but I'll > send patches and see what you think. > >> Steffen, any insight on this? What would be a good way to support >> both Maven and passwd files as data sources? >> >> Cheers, >> Richard >> > > Thanks for your time. > > Regards, > James > -- > James Ring |
From: James R. <sj...@jd...> - 2005-11-30 22:43:45
|
Hi Richard, > > Yeah, the name sucks. I think the class should not represent a single > profile, but a method of getting author profiles. It's clear what the > implementation classes would be, things like PasswdFile, > MavenProjectFile, SourceforgeWebsiteScraper. Now find a good common > name for these. > > AuthorProfileSource? > AuthorProfileFactory? This sounds like the classic situation for a factory pattern. You could have the Factory as an abstract class with pretty much the single method public abstract AuthorProfile getProfile( Author a ); and then just subclass it and provide the correct concrete implementation depending on the command line options. Alternatively, you could it as a Factory which calls other factories. For example, AuthorProfileFactory may call all its "sub-factories" (PasswdAuthorProfileFactory) until it finds a factory which can give back a profile for the given author. This would allow StatCVS to source its author profiles from many different sources at the same time... Whether that's useful or not is another question... :) > > Imagine that profiles may provide other things except real names. > Email address, web link etc. There's no need to duplicate all these > methods on both the Author and Profiles class. Agreed. It was a silly suggestion on my part. > Also, I see no need why the Author class should know about the > existence of the profiles. I want the Author class to remain as close > as possible to the concept of a CVS login. We're discussing a > presentation issue, and changing stuff on the presentation layer > shouldn't affect the core CVS classes. Actually, the AuthorProfile shouldn't have anything to do with the presentation. How you choose to represent the author's profile is totally up to the presentation layer. AuthorProfiles should be concerned only with those things which make up an author's profile (like you said, an e-mail address, URL, etc). Presentation layer classes would merely need to call Author.getProfile() and then render that profile any way they like. > (Actually, the Author class should probably be called CVSLogin. My > suggestions may make more sense if seen from that point of view.) Ok, maybe I am thinking differently about the role of the Author class. > Richard > Regards, James PS. I'm now using StatCVS at my work, and the bossman likes it a lot ;) -- James Ring |
From: Richard C. <ri...@cy...> - 2005-11-30 23:50:14
|
Hi James, On 30 Nov 2005, at 23:43, James Ring wrote: > Hi Richard, > >> >> Yeah, the name sucks. I think the class should not represent a single >> profile, but a method of getting author profiles. It's clear what the >> implementation classes would be, things like PasswdFile, >> MavenProjectFile, SourceforgeWebsiteScraper. Now find a good common >> name for these. >> >> AuthorProfileSource? >> > > AuthorProfileFactory? This sounds like the classic situation for a > factory > pattern. This implies that you have an AuthorProfile class with many instances. Can we do without? > You could have the Factory as an abstract class with pretty much > the single > method > > public abstract AuthorProfile getProfile( Author a ); > > and then just subclass it and provide the correct concrete > implementation > depending on the command line options. > > Alternatively, you could it as a Factory which calls other factories. > For example, AuthorProfileFactory may call all its "sub-factories" > (PasswdAuthorProfileFactory) until it finds a factory which can > give back > a profile for the given author. This would allow StatCVS to source its > author profiles from many different sources at the same time... > Whether > that's useful or not is another question... :) Take it easy here ... ;-) >> Also, I see no need why the Author class should know about the >> existence of the profiles. I want the Author class to remain as close >> as possible to the concept of a CVS login. We're discussing a >> presentation issue, and changing stuff on the presentation layer >> shouldn't affect the core CVS classes. > > Actually, the AuthorProfile shouldn't have anything to do with the > presentation. You're right. Presentation was the wrong term. > How you choose to represent the author's profile is totally > up to the presentation layer. AuthorProfiles should be concerned > only with > those things which make up an author's profile (like you said, an e- > mail > address, URL, etc). > > Presentation layer classes would merely need to call > Author.getProfile() > and then render that profile any way they like. That's where I disagree. I'd prefer the log parsing and representation to make no assumptions about the application that uses the data. Having Author.getProfile() would add such an assumption to the log representation. I want to be able to re-use the low-level classes (like Author) without having a dependency that forces me to drag higher-level classes (like the Profile stuff) into my new project. Hope that makes sense. Richard > >> (Actually, the Author class should probably be called CVSLogin. My >> suggestions may make more sense if seen from that point of view.) > > Ok, maybe I am thinking differently about the role of the Author > class. > >> Richard >> > > Regards, > James > > PS. I'm now using StatCVS at my work, and the bossman likes it a > lot ;) > -- > James Ring > > > > ------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. Do you grep through > log files > for problems? Stop! Download the new AJAX search engine that makes > searching your log files as easy as surfing the web. DOWNLOAD > SPLUNK! > http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click > _______________________________________________ > Statcvs-users mailing list > Sta...@li... > https://lists.sourceforge.net/lists/listinfo/statcvs-users > |
From: Steffen P. <ste...@gm...> - 2005-12-01 07:38:12
|
Hi, > > AuthorProfileFactory? This sounds like the classic situation for a > > factory > > pattern. > > This implies that you have an AuthorProfile class with many > instances. Can we do without? Why would we want to? Creating these objects is a one time task, just as parsing the cvs log file, and should be done on start up. IMHO a simple Profile class with a couple of getters and setters will do the job. But while we are at it, we may want to go with an interface that specifies the get methods and provide a default implementation that has the setters as well. > > Presentation layer classes would merely need to call > > Author.getProfile() > > and then render that profile any way they like. > > That's where I disagree. I'd prefer the log parsing and > representation to make no assumptions about the application that uses > the data. Having Author.getProfile() would add such an assumption to > the log representation. Yes, I certainly agree. The profile stuff should be kept separate from the model code as there is no apparent connection to the cvs log. > I want to be able to re-use the low-level > classes (like Author) without having a dependency that forces me to > drag higher-level classes (like the Profile stuff) into my new project. Exactly. StatCvs-XML which shares the model code has a very different approach to the profile . For maven we have a straight forward dom parser that stores results in a map of maps. SAXBuilder builder = new SAXBuilder(); Document suite = builder.build(file); Element element = suite.getRootElement().getChild("developers"); if (element != null) { for (Iterator it = element.getChildren().iterator(); it.hasNext();) { Element developer = (Element)it.next(); String id = developer.getChildText("id"); if (id == null) { continue; } if (developer.getChildText("name") != null) { settings.setFullname(id, developer.getChildText("name")); } if (developer.getChildText("image") != null) { settings.setAuthorPic(id, developer.getChildText("image")); } else if (developer.getChildText("url") != null) { settings.setAuthorPic(id, developer.getChildText("url") + "/" + id + ".png"); } } } If anyone were to define a profile class I would probably go ahead and refactor the code though :). Steffen |
From: Richard C. <ri...@cy...> - 2005-12-01 16:49:21
|
On 1 Dec 2005, at 01:31, Steffen Pingel wrote: > Hi, > >>> AuthorProfileFactory? This sounds like the classic situation for a >>> factory >>> pattern. >> >> This implies that you have an AuthorProfile class with many >> instances. Can we do without? > > Why would we want to? Creating these objects is a one time task, > just as > parsing the cvs log file, and should be done on start up. IMHO a > simple > Profile class with a couple of getters and setters will do the job. OK, you win. Let's have an AuthorProfile type. > But while > we are at it, we may want to go with an interface that specifies > the get > methods and provide a default implementation that has the setters > as well. Nah. I don't see why an interface should be necessary at the current state. I don't want to generalize prematurely. Would that cover your needs? interface AuthorProfileFactory Profile getProfile(String loginName); class AuthorProfile String getFullName(); String getImageURL(); String getWebPageURL(); + setters Richard >>> Presentation layer classes would merely need to call >>> Author.getProfile() >>> and then render that profile any way they like. >> >> That's where I disagree. I'd prefer the log parsing and >> representation to make no assumptions about the application that uses >> the data. Having Author.getProfile() would add such an assumption to >> the log representation. > > Yes, I certainly agree. The profile stuff should be kept separate > from the > model code as there is no apparent connection to the cvs log. > >> I want to be able to re-use the low-level >> classes (like Author) without having a dependency that forces me to >> drag higher-level classes (like the Profile stuff) into my new >> project. > > Exactly. StatCvs-XML which shares the model code has a very > different approach > to the profile . For maven we have a straight forward dom parser > that stores > results in a map of maps. > > SAXBuilder builder = new SAXBuilder(); > Document suite = builder.build(file); > Element element = suite.getRootElement().getChild("developers"); > if (element != null) { > for (Iterator it = element.getChildren().iterator(); it.hasNext > ();) { > Element developer = (Element)it.next(); > String id = developer.getChildText("id"); > if (id == null) { > continue; > } > if (developer.getChildText("name") != null) { > settings.setFullname(id, developer.getChildText("name")); > } > if (developer.getChildText("image") != null) { > settings.setAuthorPic(id, developer.getChildText > ("image")); > } > else if (developer.getChildText("url") != null) { > settings.setAuthorPic(id, developer.getChildText > ("url") + "/" + > id + ".png"); > } > } > } > > If anyone were to define a profile class I would probably go ahead and > refactor the code though :). > > Steffen > > > ------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. Do you grep through > log files > for problems? Stop! Download the new AJAX search engine that makes > searching your log files as easy as surfing the web. DOWNLOAD > SPLUNK! > http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click > _______________________________________________ > Statcvs-users mailing list > Sta...@li... > https://lists.sourceforge.net/lists/listinfo/statcvs-users > |
From: James R. <sj...@jd...> - 2005-12-02 00:14:23
|
Hi all, > > OK, you win. Let's have an AuthorProfile type. > [ ... snip ... ] > Would that cover your needs? > > interface AuthorProfileFactory > Profile getProfile(String loginName); Not wanting to be overly picky, but I think it should be an abstract class. This is because you may want to add some behaviour to the factory itself later down the track (like what I was saying about multiple author profile sources, just as an example). Also, I think it should be public abstract Profile getProfile( Author a ); Maybe, (again in the future) there are things about an Author apart from the login name which can help the factory build the author's profile. > class AuthorProfile > String getFullName(); > String getImageURL(); > String getWebPageURL(); > + setters Looks good to me! This just leaves the question of where these instances will be kept. I still think Author.getProfile() is the best option. If this doesn't make sense because the Author class actually represents something other than the person making the changes to the repository, then why not call it something else and introduce the Author concept elsewhere? > Richard Regards, James -- James Ring |
From: Richard C. <ri...@cy...> - 2005-12-02 08:36:28
|
On 2 Dec 2005, at 01:14, James Ring wrote: > Hi all, > >> >> OK, you win. Let's have an AuthorProfile type. >> [ ... snip ... ] >> Would that cover your needs? >> >> interface AuthorProfileFactory >> Profile getProfile(String loginName); > > Not wanting to be overly picky, but I think it should be an > abstract class. > This is because you may want to add some behaviour to the factory > itself > later down the track (like what I was saying about multiple author > profile > sources, just as an example). That may be true, but let's turn the interface into an abstract class later down the track, when/if that common code actually emerges. > Also, I think it should be > > public abstract Profile getProfile( Author a ); > > Maybe, (again in the future) there are things about an Author apart > from the > login name which can help the factory build the author's profile. Yes, good point. Thanks. >> class AuthorProfile >> String getFullName(); >> String getImageURL(); >> String getWebPageURL(); >> + setters > > Looks good to me! This just leaves the question of where these > instances > will be kept. I still think Author.getProfile() is the best option. > If this > doesn't make sense because the Author class actually represents > something > other than the person making the changes to the repository, then > why not > call it something else and introduce the Author concept elsewhere? I'd like the design to be driven by code and features. So far, I have seen no evidence that any feature or code would require or benefit from the introduction of such a new class. Everything we want to do right now can be done in a reasonably clear and duplicate-free way using just the current Author (more like CVSLogin) and the new AuthorProfile classes. Let's introduce a new concept when/if some feature calls for it. Cheers, Richard > >> Richard > > Regards, > James > -- > James Ring > > > > ------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. Do you grep through > log files > for problems? Stop! Download the new AJAX search engine that makes > searching your log files as easy as surfing the web. DOWNLOAD > SPLUNK! > http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click > _______________________________________________ > Statcvs-users mailing list > Sta...@li... > https://lists.sourceforge.net/lists/listinfo/statcvs-users > |
From: Steffen P. <ste...@gm...> - 2005-12-02 08:45:04
|
Hi, > Nah. I don't see why an interface should be necessary at the current > state. I don't want to generalize prematurely. > > Would that cover your needs? > > interface AuthorProfileFactory > Profile getProfile(String loginName); > > class AuthorProfile > String getFullName(); > String getImageURL(); > String getWebPageURL(); > + setters Sound cool. Now someone has to write the patch :). Steffen -- Steffen Pingel - ste...@gm... - http://steffenpingel.de |