You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@roller.apache.org by Dave <sn...@gmail.com> on 2009/02/04 17:46:58 UTC
Re: Introduction and patches
On Thu, Jan 29, 2009 at 6:45 AM, Emmanouil Batsis (Manos)
<ma...@abiss.gr> wrote:
> Just wanted to introduce myself to the list and let you know I added a
> couple of patches to jira, namely
>
> * SiteModel.getNewWeblogs javadoc fix, functionality improovement and patch
> [1]
> * Add "named" pin support[2]
>
> I'm rather errr, new to JPA, struts2 and velocity so these may not be of the
> greatest quality etc. The "named pin" is rather incomplete for inclusion as
> it only does it for JPA (VS hibernate) and mysql. It's design is also
> suboptimal, e.g. it adds a new column to the weblogentry table VS a new
> table (should be null in most rows).
>
> However, I'm willing to work more on these (or any other patches we submit),
> as well as a new or modified main template to include this in a future
> release if there's enough interest.
>
> [1] http://issues.apache.org/roller/browse/ROL-1780
> [2] http://issues.apache.org/roller/browse/ROL-1786
OK, I finally got a chance to take a look at these patches.
Unfortunately, I ran into a couple of issues:
1) There is a local path built into the patches "/Users/manos/lib/roller_4.1"
I get the message "patch cannot be applied in this context" when I
attempt to apply them against either the trunk or the 4.0 branch. I
create my patches with "svn diff > patchfile.txt" and it creates
relative instead of absolute paths.
2) They are made against the Roller 4.0 code base
These are new features and as such, should really go into the trunk.
If you resubmit and fix at least item #1, I'll take another look.
Thanks,
- Dave
Re: Introduction and patches
Posted by Dave <sn...@gmail.com>.
On Fri, Feb 6, 2009 at 1:42 PM, Emmanouil Batsis (Manos) <ma...@abiss.gr> wrote:
>> 1) There is a local path built into the patches
>> "/Users/manos/lib/roller_4.1"
>> I get the message "patch cannot be applied in this context" when I
>> attempt to apply them against either the trunk or the 4.0 branch. I
>> create my patches with "svn diff > patchfile.txt" and it creates
>> relative instead of absolute paths.
>
>> 2) They are made against the Roller 4.0 code base
>> These are new features and as such, should really go into the trunk.
>
>
> Done [1] for the trunk, but couldn't update "Affects Version/s:" in jira.
>
> Also, I dunno why eclipse insists using absolute paths in the patches. I
> modified all these in the new patch and then tried to apply the patch in
> roller project's basedir; works OK.
>
> [1] https://issues.apache.org/roller/browse/ROL-1786
Sorry about the delay. I am looking at this now and hope to have some
more comments very soon.
- Dave
Re: Introduction and patches
Posted by Dave <sn...@gmail.com>.
On Mon, Feb 23, 2009 at 11:40 AM, Emmanouil Batsis (Manos)
<ma...@abiss.gr> wrote:
> Dave wrote:
>>
>> I would much prefer to see this feature implemented like so:
>>
>> 1) add a new Global Entry Management page, like the existing Global
>> Comment Management page, to the Server Admin section of the UI.
>>
>> 2) In that new page, an admin user would be able to set and unset the
>> Pinned to Main flag for any entry by setting a check box.
>>
>> 3) Add a new argument to the getWeblogEntriesPinnedToMain() method so
>> that a tag can be specified when querying for pinned entries. Do the
>> same thing for the corresponding method in the SiteModel.
>>
>> With this approach, a site administrator could setup a front-page blog
>> with different sections for "featured" entries and each section would
>> be assigned a tag.
>
> Can you please provide an example of the new method signature and it's
> behavior? E.g. will it only return a set of the most recent entry per tag?
Right. The signature would be:
List getWeblogEntriesPinnedToMain(String tag, int count)
And it would return only entries that match the specified tag.
- Dave
Re: Introduction and patches
Posted by "Emmanouil Batsis (Manos)" <ma...@abiss.gr>.
Dave wrote:
> I would much prefer to see this feature implemented like so:
>
> 1) add a new Global Entry Management page, like the existing Global
> Comment Management page, to the Server Admin section of the UI.
>
> 2) In that new page, an admin user would be able to set and unset the
> Pinned to Main flag for any entry by setting a check box.
>
> 3) Add a new argument to the getWeblogEntriesPinnedToMain() method so
> that a tag can be specified when querying for pinned entries. Do the
> same thing for the corresponding method in the SiteModel.
>
> With this approach, a site administrator could setup a front-page blog
> with different sections for "featured" entries and each section would
> be assigned a tag.
Can you please provide an example of the new method signature and it's
behavior? E.g. will it only return a set of the most recent entry per tag?
--
Manos Batsis, Chief Technologist
__ _
____ _/ /_ (_)_________ ____ ______
/ __ `/ __ \/ / ___/ ___// __ `/ ___/
/ /_/ / /_/ / (__ |__ )/ /_/ / /
\__,_/_.___/_/____/____(_)__, /_/
/____/
http://www.Abiss.gr
19, Andrea Kalvou Street,
14231, Nea Ionia,
Athens, Greece
Tel: +30 211-1027-900
Fax: +30 211-1027-999
Re: Introduction and patches
Posted by Dave <sn...@gmail.com>.
On Fri, Feb 6, 2009 at 1:42 PM, Emmanouil Batsis (Manos) <ma...@abiss.gr> wrote:
> Dave wrote:
>
>> 1) There is a local path built into the patches
>> "/Users/manos/lib/roller_4.1"
>> I get the message "patch cannot be applied in this context" when I
>> attempt to apply them against either the trunk or the 4.0 branch. I
>> create my patches with "svn diff > patchfile.txt" and it creates
>> relative instead of absolute paths.
>
>> 2) They are made against the Roller 4.0 code base
>> These are new features and as such, should really go into the trunk.
>
>
> Done [1] for the trunk, but couldn't update "Affects Version/s:" in jira.
>
> Also, I dunno why eclipse insists using absolute paths in the patches. I
> modified all these in the new patch and then tried to apply the patch in
> roller project's basedir; works OK.
>
> [1] https://issues.apache.org/roller/browse/ROL-1786
Thank you very much for submitting this patch. I apologize for taking
so long to review it.
Over the weekend, I took a close look and found that this patch:
a) allows any user to create a "named pin" for a weblog entry, a
unique string that identifies the weblog entry across the entire site.
If one user adds a named pin "foo" to a published weblog entry and
then a second user does the same, the "foo" pin will be removed from
the first user's entry and added to the second user's entry. The code
does not care about named pin collisions except for published entries.
b) adds a text field to the weblog entry add and edit pages, right
below the title where a user can add a named pin for an entry.
c) adds method WeblogEntryManager.getPinnedEntres(String status,
Integer max) so that one can query for pinned entries with a specific
name.
d) adds a method SiteModel.getNamedPinEntryMap() so that named pin
entries can be retrieved (see the screenshot attached to the issue for
an example use case).
It seems to me that this is a cumbersome way to allow a site
administrator to pick entries to be featured in specific locations in
the front page of a site. A site administrator will have to edit the
individual entries that he wishes to feature on the main page, instead
of nominating entries from a central UI. The named pin field clutters
the entry add and edit page with a new field that is not useful and
confusing to most users. It also requires that a new field be added to
weblog entry table.
I would much prefer to see this feature implemented like so:
1) add a new Global Entry Management page, like the existing Global
Comment Management page, to the Server Admin section of the UI.
2) In that new page, an admin user would be able to set and unset the
Pinned to Main flag for any entry by setting a check box.
3) Add a new argument to the getWeblogEntriesPinnedToMain() method so
that a tag can be specified when querying for pinned entries. Do the
same thing for the corresponding method in the SiteModel.
With this approach, a site administrator could setup a front-page blog
with different sections for "featured" entries and each section would
be assigned a tag.
Users who wish to have their entries featured would know to use those
tags on their entries. Every day or week an admin user would login to
Roller to decide which entries should be featured. He would use the
Global Entry Management page, filter by each the featured tags and
choose the entries to be featured by setting the pinned-to-main
checkfox for entries he selected.
Each featured section in the front page theme could be setup to show
only the most recent 1 or N pinned to main entries in the tag that is
assigned to the section. We might want to rename the "pinned-to-main"
field to "featured" at least in the UI to make it more clear what the
purposes is.
That way we would not need to add a new database field, we would not
need to add additional UI to the entry add/edit page and the site
administrator would not have to edit individual entries to pin them to
the main page of the site.
Does that make sense?
- Dave
Re: Introduction and patches
Posted by "Emmanouil Batsis (Manos)" <ma...@abiss.gr>.
Dave wrote:
> 1) There is a local path built into the patches "/Users/manos/lib/roller_4.1"
> I get the message "patch cannot be applied in this context" when I
> attempt to apply them against either the trunk or the 4.0 branch. I
> create my patches with "svn diff > patchfile.txt" and it creates
> relative instead of absolute paths.
> 2) They are made against the Roller 4.0 code base
> These are new features and as such, should really go into the trunk.
Done [1] for the trunk, but couldn't update "Affects Version/s:" in jira.
Also, I dunno why eclipse insists using absolute paths in the patches. I
modified all these in the new patch and then tried to apply the patch in
roller project's basedir; works OK.
[1] https://issues.apache.org/roller/browse/ROL-1786
--
Manos Batsis, Chief Technologist
__ _
____ _/ /_ (_)_________ ____ ______
/ __ `/ __ \/ / ___/ ___// __ `/ ___/
/ /_/ / /_/ / (__ |__ )/ /_/ / /
\__,_/_.___/_/____/____(_)__, /_/
/____/
http://www.Abiss.gr
19, Andrea Kalvou Street,
14231, Nea Ionia,
Athens, Greece
Tel: +30 210 2851517
Re: Introduction and patches
Posted by "Emmanouil Batsis (Manos)" <ma...@abiss.gr>.
Dave wrote:
> If you resubmit and fix at least item #1, I'll take another look.
Ha, great timing. I just checked out the trunk to apply my changes on it
(need to use the Media Blogging features ;-)
Cheers,
Manos
--
Manos Batsis, Chief Technologist
__ _
____ _/ /_ (_)_________ ____ ______
/ __ `/ __ \/ / ___/ ___// __ `/ ___/
/ /_/ / /_/ / (__ |__ )/ /_/ / /
\__,_/_.___/_/____/____(_)__, /_/
/____/
http://www.Abiss.gr
5, Daphnidos Street,
14122, Neo Iraklio,
Athens, Greece
Tel: +30 210 2851517
http://www.linkedin.com/in/manosbatsis