Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add project releases RSS feed #7013

Merged
merged 17 commits into from
May 1, 2020
Merged

Add project releases RSS feed #7013

merged 17 commits into from
May 1, 2020

Conversation

scop
Copy link
Contributor

@scop scop commented Nov 17, 2019

Closes #1683

Once #7012 is in, this implementation should be amended with authors in items as well. (it's in now)

@yeraydiazdiaz
Copy link
Contributor

@scop #7012 has been merged, would you like to bring this PR up to date?

@scop
Copy link
Contributor Author

scop commented Jan 19, 2020

Sure, done now: rebased to master and added authors to this feed, too.

warehouse/routes.py Outdated Show resolved Hide resolved
@di di mentioned this pull request Jan 21, 2020
Copy link
Contributor

@yeraydiazdiaz yeraydiazdiaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @scop! Just needs a merge conflict resolved.

@scop
Copy link
Contributor Author

scop commented Jan 27, 2020

Just needs a merge conflict resolved.

Done.

@scop
Copy link
Contributor Author

scop commented Feb 6, 2020

Anything more I can do to get this in?

@scop
Copy link
Contributor Author

scop commented Feb 22, 2020

@di @yeraydiazdiaz Ping, anything more I can do to get this in? The approval was posted about a month ago already.

@scop
Copy link
Contributor Author

scop commented Apr 25, 2020

@di @yeraydiazdiaz ping...

Copy link
Member

@di di left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Only a few small comments.

Also, it would probably be good to add a link to the the RSS feed (with an RSS icon as well) to this link:

Screen Shot 2020-05-01 at 12 02 27 PM

Something like:

<h2 class="page-title split-layout">
  <span>Release history</span>
  <span class="reset-text margin-top">
    <a href="/help/#project-release-notifications">Release notifications</a> | <a href="{{ request.route_path('rss.project.releases', name=release.project.normalized_name) }}">RSS Feed <i class="fa fa-rss" aria-hidden="true"></i></a>
    </span>
</h2>

Which would look like:

Screen Shot 2020-05-01 at 12 14 14 PM

warehouse/rss/views.py Outdated Show resolved Hide resolved
warehouse/templates/base.html Outdated Show resolved Hide resolved
warehouse/templates/base.html Outdated Show resolved Hide resolved
warehouse/templates/packaging/detail.html Outdated Show resolved Hide resolved
tests/unit/rss/test_views.py Outdated Show resolved Hide resolved
tests/unit/rss/test_views.py Outdated Show resolved Hide resolved
@di
Copy link
Member

di commented May 1, 2020

Also, you'll need to run make translations and commit the changes to get CI to pass here.

docs/api-reference/feeds.rst Outdated Show resolved Hide resolved
scop and others added 7 commits May 1, 2020 21:09
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
@di
Copy link
Member

di commented May 1, 2020

@scop For future reference, it's possible to batch all suggestions into a single commit: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/incorporating-feedback-in-your-pull-request

Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
@scop
Copy link
Contributor Author

scop commented May 1, 2020

Yes, I noticed that option and would have definitely used it. However the button stayed disabled and grayed out so I couldn't.

@scop
Copy link
Contributor Author

scop commented May 1, 2020

Applied suggestions here, thanks. I trust you'll squash/tidy up the history on eventual merge :)

It's been so long since I've touched this project that I'll need to go and refresh my memory how I can actually run it to test these, and will take a look at the translations and possible other CI business then.

tests/unit/rss/test_views.py Outdated Show resolved Hide resolved
@di
Copy link
Member

di commented May 1, 2020

However the button stayed disabled and grayed out so I couldn't.

I think you have to switch to the "Files changed" view to get that to work, kinda weird.

@di
Copy link
Member

di commented May 1, 2020

@scop I'll save you the trouble 🙂

@di di merged commit e8a1388 into pypi:master May 1, 2020
@brainwane
Copy link
Contributor

@scop Thank you for this!

Just in case you are interested in working on further Warehouse stuff: the next big leap that needs to happen in Warehouse's APIs and feeds is #7730. If you are interested in helping out with that, please comment there.

@scop
Copy link
Contributor Author

scop commented May 2, 2020

Thanks! I see the actual feeds and help content are live, however the page content changes are not (to me at least): the <link> is not there yet, neither is the RSS feed link in the Release history tab. Caching going on somewhere?

@di
Copy link
Member

di commented May 2, 2020

@scop Yes, they will slowly appear as the pages fall out of the cache and get re-rendered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package update feeds
4 participants