Additional comments from Committers meeting on Nov 18, 2009.
Summary: We came to an agreement that Larry's patch looks good as-is for 1.6. However, there are some addition features/suggestions we may want to look into post-1.6. The discussion below is long, but there are some good ideas for future additions that came up.
[15:16] <StuartLewis_> And the second patch is:
[15:17] <StuartLewis_> http://jira.dspace.org/jira/browse/DS-377
Add META tags identifying DSpace source version to Web UIs
[15:17] <StuartLewis_> See the comments in this one, especially Larry's last comment to explain how the version will be identified.
[15:18] <mdiggory> Looking at the topic...
[15:18] <tdonohue> +1 sounds good to me - glad we can populate based on version in Maven POM
[15:18] <mhwood> +1
[15:18] <mdiggory> A maven based approach would use filtering in the mvn package phase
[15:19] <kshepherd> +1
[15:19] <mdiggory> you don't really need to java code this
[15:20] <tdonohue> mdiggory: does this mean you disagree with Larry's approach?
[15:20] <kshepherd> hmm.. needs more discussion?
[15:20] <mdiggory> identify the files you want to filter and create a parameter in them, map that parameter in the pom.xml
[15:20] <mdiggory> on release that parameter will be processed
[15:21] <mdiggory> Seems like its coding before researching
[15:21] <tdonohue> mdiggory: so you are saying you'd list the specific files in XMLUI or JSPUI that need a parameter to be replaced?
[15:21] <mdiggory> pretty much
[15:21] <tdonohue> and the maven will replace it during the packaging?
[15:21] <mhwood> So you want the package phase to customize the XSL or something? It'd have to track down all a site's themes....
[15:21] <mdiggory> war generation includes filtering
[15:22] <tdonohue> would it also work for completely custom themes, like mhwood brings up?
[15:22] <mdiggory> I assume you would run this risk if you were overriding that header in dri2xhtml
[15:23] <jeffreytrimble> We'd have to warn users about this....so they remember to re-customize those files affected by this.
[15:24] <mdiggory> but i will add... theres no reason this couldn't be in a properties file or the default Navigation transformer rather than chasing individual xslt templates
[15:24] <mhwood> It's being fetched from a properties file now.
[15:24] <jeffreytrimble> But it is entirely possible to gather up a list of files that this change would/does affect and let the users know the work they have in store?
[15:24] <mdiggory> in which case, it might not even be maven based filtering... just use SVN versioning infor
[15:25] <mdiggory> svn keywords in the java classes
[15:25] <mdiggory> or a properties file
[15:25] <bradmc> Seems fragile to have it scattered among customizable files. An include of a single file that gets changed seems more robust - but creates other issues.
[15:25] <tdonohue> jeffreytrimble: yes, you are right...we should warn users about this either way...and i think once we've finalized how to do this, we can get you a list of affected files
[15:25] <mdiggory> int he classpath for xmlui-api, dspace-api or jspui-api
[15:25] * StuartLewis__ (n=StuartLe@188.8.131.52) has joined #duraspace
[15:26] <mhwood> If sites have to be warned about such matters...do we really want to do that to them?
[15:26] <jeffreytrimble> let's not use the word warning...let just say they need to be aware that the upgrade has some changes they may need to address in customizations.
[15:27] <tdonohue> bradmc +1 changing in one place is probably best.
[15:27] <mdiggory> doesn't need to be perfect... you cannot guarantee UI html serialization behavior... another option might be response headers?
[15:28] <mdiggory> which could be theme independent
[15:28] <tdonohue> seems like we are over thinking this....to be honest, Larry's patch is rather small
[15:29] <tdonohue> and maybe there is some use to being able to get that version via a Java call
[15:29] <mdiggory> not really, All I'm talking about in this case is one line in the servlets that spits out the build number, replaced using svn keyword substitution
[15:29] <tdonohue> maybe eventually one could run "dspace --version" to get a report of what version they are running, etc
[15:30] <mdiggory> seems like a job for the XMLUI Control Panel
[15:30] <mhwood> I kinda want to use the same information in a human-readable place, like: "This is DSpace 1.6.0, ScholarWorks build 123"
[15:31] <bradmc> like having dspace validate that it's code matches it's database schema? Couldn't do that at the UI level.
[15:31] <snail1> it would also be useful to have the version in an HTML comment on the homepage by default
[15:32] <StuartLewis__> snail1: It was that suggestion that kicked-off this discussion :)
[15:32] <mdiggory> downfall with svn keyword approach is that it might change across repos where the code is being duplicated (yuck)
[15:32] <kshepherd> the original patch added a meta tag
[15:32] <kshepherd> but the discussion's gone a bit over my head since then ;)
[15:33] <mdiggory> can add meta tag, can also add response header... both relatively easy, first is UI dependent, second can be deeper in the content generation layer
[15:33] <mdiggory> big question, what is wanted to be known by "us" when looking at a DSpace instance
[15:34] <mhwood> I think it boils down to whether the cost of keeping multiple copies in sync. outweighs the cost of going to a single definitive copy every time you want it.
[15:34] <mdiggory> as someone troubleshooting DSpace I want to know (1) version (2) build (3) build date (4) by whom
[15:34] <tdonohue> so, how do we come to a consensus here? mdiggory are you still against Larry's approach?
[15:35] <mdiggory> Yes I'm against this code solution
[15:36] <mdiggory> hmmm.... wait
[15:36] <mdiggory> let me review it just a little more.
[15:36] * bradmc muses on bikesheds
[15:37] <kshepherd> i'm a bit worried about complicating things too much for a small feature that was an idea /we/ had rather than the community-at-large
[15:38] <mdiggory> ok... no I am not against this approach.
[15:38] <mdiggory> I will add
[15:38] <mdiggory> that if you want "build" specific customization...
[15:39] <tdonohue> ok, so do we want to revote to (1) start with Larry's approach for 1.6...after 1.6 (2) we can review it and see if it needs to be tweaked to provide more info (like kshepherd suggests)
[15:39] <mdiggory> generalize this slightly to (1) return the properties as well as string and (2) add ability to lookup pom by package
[15:40] <StuartLewis__> +1 Larry's solution, look at improving it later if required.
[15:40] <mdiggory> the mhwood can add his modules/xmlui/pom.xml properties for his particular build as a second property
[15:40] <mdiggory> in that packages pom.xml pom.properties
[15:42] <mdiggory> recommend adding call to this class in DSpaceServlet and/or appropriate class in XMLUI to support setting response headers
[15:42] * StuartLewis_ (n=StuartLe@184.108.40.206) Quit (Read error: 110 (Connection timed out))
[15:42] <tdonohue> mdiggory: I think you are starting into a different feature request. This feature was just a recommendation for a simple HTML <Meta> tag to identify the version of DSpace you are running
[15:43] <tdonohue> but, I think we could add a new Jira request for some of the features you are talking about
[15:43] <mdiggory> no, XMLUI themes may have overridden the creation of html head meta
[15:43] <mdiggory> sticking it into the response header assures robustness
[15:43] <tdonohue> mdiggory: true...but ast jeffreytrimble said, we can let users know that in the upgrade docs
[15:44] <mdiggory> we see this usefull for different reasons
[15:44] <mdiggory> response header requires no documentation caveats
[15:44] <jeffreytrimble> On the UI side, it's good to know what version a site is running....
[15:44] <tdonohue> yea, i can agree with that. That's part of why I wonder if this needs to be a separate Jira issue that we can vote on separately
[15:44] <jeffreytrimble> on the admin side, it's good to know the version and the build.
[15:45] <mdiggory> this is both cases
[15:45] <StuartLewis__> But joe-average-dspace-repo-manager doedn't know how to interrogate http headers
[15:45] <mhwood> I think the code as it stands can be used for several quite different purposes, including the original one. A separate issue could then evolve it as needed, later.
[15:45] <jeffreytrimble> really? My users could care less what build we are running...let alone versions...but all of us here today would be interested in at least the version
[15:46] <mdiggory> This is not for average joe
[15:46] <mdiggory> this is for webdeveloper troublshooter
[15:46] <mdiggory> developer administrator
[15:46] * tdonohue notices we've been on this same issue for 30+ minutes
[15:46] <StuartLewis__> Well it might be - if we tell them 'view the html source and tell us what is says at GENERATOR='
[15:46] <mhwood> And my supervisor, who regularly asks, "what versions are we running now?"
[15:46] <mdiggory> no, I say... send me a link to your site and I look
[15:47] <tdonohue> ok, i'd like to suggest we agree to disagree on this, and just choose something for 1.6. It can always be tweaked for 1.6.1 :)
[15:48] <mdiggory> you can always put it in the control panel or present it in the footer if you have such interested supervisors
[15:49] <mdiggory> agreed, at least we agree to add what is there
[15:49] <tdonohue> do we want to revote on Larry's suggestion for 1.6? And then add a new Jira issue with these extra suggestions (so that we can track them -- some good thoughts here)
[15:49] <kshepherd> having something simply for troubleshooting by developers sounds cool, but it can't be something that makes extra work for IR admins in terms of themes, etc...
[15:49] <kshepherd> especially since they're not the ones asking for it ;)
[15:50] <mdiggory> just tack them into the JIRA issue thats there... commit the patch and say... if you want to do more, continue to use this JIRA ticket.
[15:50] <mhwood> That's why I worry that I keep seeing, "we need to document how we're going to break your themes"
[15:50] <kshepherd> mhwood: yeah
[15:50] <mdiggory> themes not broken...
[15:50] <tdonohue> Ok, I'm calling for a revote on: http://jira.dspace.org/jira/browse/DS-377
as-is for 1.6.
[15:50] <mhwood> Feature broken unless you re-customize?
[15:50] <tdonohue> +1
[15:51] <kshepherd> but in this case, nothing 'broken', just.. it won't work without extra effort on those custom themes
[15:51] <mdiggory> nothing being done here is non-backward or forwards compatable
[15:51] <mhwood> +1
[15:51] <mdiggory> using response header is just more reliable given chance that its been customized out of someones theme
[15:51] <mdiggory> +1
[15:52] <kshepherd> +1
[15:52] <snail1> +1
[15:52] <StuartLewis__> We can do headers too if we get a patch