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

Feature: Add setting to append ref parameter to links in feeds #3249

Merged
merged 3 commits into from
Apr 19, 2024

Conversation

GiovanH
Copy link
Contributor

@GiovanH GiovanH commented Nov 22, 2023

This is a feature I'm experimenting with for personal use, as it's useful for analytics purposes to see if people are using the rss feeds.

  • Ensured tests pass and (if applicable) updated functional test output
  • Conformed to code style guidelines by running appropriate linting tools
  • Added tests for changed code
  • Updated documentation for changed code

This is more of a proposal at this stage. Is this something people would be interested in?

@GiovanH
Copy link
Contributor Author

GiovanH commented Jan 16, 2024

Updated with documentation.

@GiovanH GiovanH marked this pull request as ready for review January 16, 2024 00:30
@justinmayer justinmayer changed the title Feature: Add rss rel param to rss feeds if requested Feature: Add setting to append RSS rel parameter to feeds Jan 16, 2024
@GiovanH
Copy link
Contributor Author

GiovanH commented Mar 3, 2024

@justinmayer Is there any further work needed here or is this ready to merge?

Copy link
Member

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

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

Hi @GiovanH. My apologies for the delay in reviewing. I've looked at this several times, but something always felt… not quite right… but I could never put my finger on what didn’t feel right.

I think I figured it out:

  1. The rel attribute is for use within HTML documents. I don't think I have ever seen a ?rel=[…] URL parameter, and I can’t find any documentation about such a parameter anywhere on the Internet. I have, on the other hand, seen ?ref=[…] URL parameters used to indicating referrer information. Perhaps you meant ref and not rel? If so, perhaps change the URL parameter, and also change the setting name to FEED_APPEND_REFERRER?
  2. For the parameter value, I suggest using the generic feed instead of rss, as a feed can be in either Atom or RSS format.

If I am wrong on any of the above counts, by all means feel free to point me to information to the contrary! 😊

@GiovanH
Copy link
Contributor Author

GiovanH commented Apr 16, 2024

I agree with you on both counts.

@justinmayer justinmayer changed the title Feature: Add setting to append RSS rel parameter to feeds Feature: Add setting to append ref parameter to feeds Apr 19, 2024
@justinmayer justinmayer changed the title Feature: Add setting to append ref parameter to feeds Feature: Add setting to append ref parameter to links in feeds Apr 19, 2024
Copy link
Member

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

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

Thank you for the feature contribution, @GiovanH! ✨

@justinmayer justinmayer merged commit 0b5934a into getpelican:master Apr 19, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants