search

scipy / SciPyCentral

SciPy Central
http://scipy-central.org
BSD 3-Clause "New" or "Revised" License
16 stars 12 forks source link

Feeds #121

Closed ksurya closed 11 years ago

ksurya commented 11 years ago

Rss 2.0 Feeds for SciPy Central. This code generates the following types of feeds

  1. /feeds/ Where we observer 30 recent submissions
  2. /feeds/<tag_name>/ we observe 30 recent submissions under <tag_name>
  3. /feeds/<entry_id>/<rev_id>/ feed for a particular entry - is ignored in the url. As in case of submissions urls, we used slug for the sake of search engines.
  4. /feeds/atom/ atom format of /feeds/

All these links are placed in respective templates. Example:

  1. On homepage,we observe [1] on the right side bar and in the footer
  2. We observer [2] on the right side bar while searching a tag /item/tag/IPython/ (example)
  3. We can observe [3] on the right side bar of submission page /item/4/1/ (example)
  4. [4] is just not shown on the template.. may be Rss is enough.

Update:

  1. All files python files in scipycentral files are trimmed using reindent.py
lomegor commented 11 years ago

I am taking a look at this, and will make comments on the diff.

lomegor commented 11 years ago

@ksurya Could you merge some of the commits? There are a lot of them. For example c769245 to 2fb7b13 could be one commit, 54e65d9 to 08da402 another one... and so on. Maybe separating between changes in the appearance and changes in the code? Also, I think all commits should be ENH, since DEV is for development tool or utility.

lomegor commented 11 years ago

I made a few comments. Overall the quality is good. Some comments are in the commit themselves when they are related to the commit message or content, and some comments are in the diff when they are related to the code itself. Make sure to catch them all.

Also, commit messages should always be written in present tense. This is because the commit itself is not something that happened, but a list of changes. (It also has the added benefit that it usually makes the messages one char or two shorter :) .)

lomegor commented 11 years ago

One more question a bit more general. Why did you decided to use objects instead of functions to show the feeds? It seems like functions would be easier to implement and understand.

ksurya commented 11 years ago

I didn't get your point.. I am sorry, is there any other way around to generate Feeds? I just followed https://docs.djangoproject.com/en/1.4/ref/contrib/syndication/

lomegor commented 11 years ago

Oh, I see! I missed that. That looks great, then.

lomegor commented 11 years ago

Ok, I think that's it. Once you correct these last things, this will be ready.

lomegor commented 11 years ago

This is ready.

pv commented 11 years ago

Ok, this seems to work great, so +1 for merging after the coding style issues are fixed.

lomegor commented 11 years ago

@pv There are multiple style errors from pep8. Including some of them in the new feeds modules. Should @ksurya fix the ones in feeds now or should we move to a later date a PR just fixing all errors from pep8?

pv commented 11 years ago

I don't think requiring full PEP8 cleanness makes much sense, just getting the basics right makes it clear enough. I don't see problems with the present code.

lomegor commented 11 years ago

Great, thanks. Ready to merge then. On 9 Jun 2013 19:10, "Pauli Virtanen" notifications@github.com wrote:

I don't think requiring full PEP8 cleanness makes much sense, just getting the basics right makes it clear enough. I don't see problems with the present code.

— Reply to this email directly or view it on GitHubhttps://github.com/scipy/SciPyCentral/pull/121#issuecomment-19170522 .