Closed (fixed)
Project:
Drupal.org customizations
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
11 Mar 2009 at 15:15 UTC
Updated:
23 May 2014 at 18:23 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
pwolanin commentedoops - can't attach a .inc - rename before adding to CVS
Comment #2
dwwMostly looks great, thanks! A few (minor) questions/issues:
A) Why is the path for the feed "security-contrib/rss.xml" when the page is "security/contrib"? Seems like security/contrib/rss.xml would be better, no? Same for security-psa
B) Why rss.xml instead of just /feed? E.g. security/contrib/feed ? I guess that's historical, e.g. for security/rss.xml right?
C) Minor typo: "this includes all module, themes ..." s/module/modules/ -- oh, and in the same spot: /s/install profiles/installation profiles/
D) Do we want "contrib" in the path or "contributions"? We tend to avoid abbreviations, although in this case, "contrib" is so wide-spread in Drupal, I don't think it's really a problem.
E) Not sure if it makes anything any easier in this case, but it seems like this could be a single view with multiple displays -- a lot of the view is the same for each one. All that's really different are the display-specific things like the menu path/title, page headers, etc, and the tid to use in the filter. I guess all that's left to save are a few bits of meta info about the view itself, and the sort handler. Ok, probably not worth it. ;)
F) "These posts by the Drupal security team are also sent to the security announcements e-mail list." -- shouldn't the "security announcements e-mail list" part be a link to join (or instructions on how)?
G) 'Security advisories for drupal core' -- ucfirst(drupal) ;)
H) For the filters, I'd use the "Node: Published or admin" filter, so that as the sec team is working on SAs, we can see unpublished SAs in the list already.
I) Do we want the "Security advisories for Drupal core" in the primary navigation menu like that? Perhaps we do, but it seems we should be sure we want that instead of getting it as a side effect of how the view menu items are configured.
J) Should we prefix all the view names with "drupalorg" to avoid namespace conflicts?
The vast bulk of this is trivial, and I'd be happy to just fix it while I commit. However, I wanted feedback on most of these before I go off and decide myself.
Yay for using views to solve our problems on d.o!
Comment #3
pwolanin commentedA/B: security/rss.xml is for historical reasons. Similarly, I left the contrib path as security-contrib/rss.xml and psa and security-psa/rss.xml since those are the aliases in place right now and I thought it friendlier not to break anyone who has already subscribed to either of those. I don't care much, however.
E: I'd guess it's a little lighter to load a View w/ one display then multiple?
I: I didn't intend for it to be in the Nav menu - not sure how to disable it other than manually.
I'll try to implement the other fixes.
Comment #4
pwolanin commentedre: F) the block on the side of the page has that info:
Comment #5
dwwRe: A/B: I think it's better to make the feed paths sane and match the path structure of the actual pages. Things are in flux right now, and we're about to do a PSA about all the changes -- I don't mind breaking a few links for folks who happen to have subscribed already. That said, I'm in favor of keeping rss.xml instead of feed so that we're internally consistent but don't break security/rss.xml which is important.
Comment #6
pwolanin commentedHere's an updated file.
Note that just before this is deployed someone should delete all the existing aliases (security/rss.xml, etc.)
Comment #7
dwwFixed a handful of problems that were still in here:
K) You were never populating the $views array in hook_views_default_views(), just reclobbering the $view object. ;)
L) We need to define hook_views_api() so views knows where to find the default views file.
M) There were some cut and paste typos where the menu titles for all 3 views said "Security advisories for Drupal core".
I also made the feed paths match the pages, and added some redirect code to prevent link rot (which we can remove whenever we want). Finally, I ported all this to the HEAD layout of drupalorg, too.
I'm going to commit and deploy these now, I'll mark this fixed once it's working on d.o.
Comment #8
dwwhttp://drupal.org/security
http://drupal.org/security/contrib
http://drupal.org/security/psa
I removed all the old aliases and I fixed up the blocks, too.
Comment #9
dwwp.s. I asked merlinofchaos about (I) (Do we want the "Security advisories for Drupal core" in the primary navigation menu like that? Perhaps we do, but it seems we should be sure we want that instead of getting it as a side effect of how the view menu items are configured.), and explained that on d.o I just disabled that menu item from the navigation menu. He said: