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.

Sign up for GitHub

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

Jump to bottom

Don't create an empty <summary> in Atom feeds #83

Merged

coreydaley merged 3 commits into gorilla:main from magical:master

Aug 17, 2023

Contributor

magical commented Aug 18, 2020 •

edited

Loading

When creating an AtomEntry from an Item, only set Summary if
item.Description is non-empty, like we already do for Content.

Currently we always emit a <summary> element, even if the item has a
non-empty <content> element. Having an empty <summary> in this case may
be confusing for feed consumers.

The Atom RFC explicitly says that a <summary> is not required in general.
(There are a couple special cases where it is required, but they aren't
relevant here.)

https://tools.ietf.org/html/rfc4287#section-4.1.1.1

Fixes #82


          Don't create an empty <summary> in Atom feeds

8946a19

When creating an AtomEntry from an Item, only set Summary if
item.Description is non-empty, like we already do for Content.

Currently we always emit a <summary> element, even if the item has a
non-empty <content> element. Having an empty <summary> in this case may
be confusing for feed consumers.

The Atom RFC explicitly says that a <summary> is not required in general.
(There are a couple special cases where it is required, but they aren't
relevant here.)

https://tools.ietf.org/html/rfc4287#section-4.1.1.1

Fixes gorilla#82

Contributor Author

magical commented Aug 26, 2020

stale bot commented Nov 1, 2020

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

stale bot added the stale label

Contributor Author

magical commented Nov 1, 2020

Not stale.

stale bot removed the stale label

stale bot commented Jan 3, 2021

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

stale bot added the stale label

Contributor Author

magical commented Jan 3, 2021

Still not stale.

stale bot removed the stale label

stale bot commented Jun 2, 2021

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

stale bot added the stale label

Contributor Author

magical commented Jun 2, 2021

Poke

stale bot removed the stale label

stale bot commented Aug 22, 2021

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

stale bot added the stale label

Contributor Author

magical commented Aug 22, 2021

Bump

stale bot removed the stale label

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

stale bot added the stale label

Contributor Author

magical commented Jan 15, 2022

poke

stale bot removed the stale label

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

stale bot added the stale label

Contributor Author

magical commented Apr 16, 2022

👋

stale bot removed the stale label

glacials commented Jun 20, 2022

The Atom RFC explicitly says that a <summary> is not required in general.
(There are a couple special cases where it is required, but they aren't
relevant here.)

This one seems maybe relevant, since the consumer can set the Type:

atom:entry elements MUST contain an atom:summary element in either
of the following cases:
...

the atom:entry contains content that is encoded in Base64; i.e., the "type" attribute of atom:content is a MIME media type [MIMEREG], but is not an XML media type [RFC3023], does not begin with "text/", and does not end with "/xml" or "+xml".

amustaque97 approved these changes

View reviewed changes

amustaque97 left a comment

Thanks for making that change. Also kudos for your effort to keep commenting on both issue and PR to avoid staleness.

cc @elithrar, merge this PR.

FelicianoTech commented Jul 9, 2023 •

edited

Loading

In case you haven't seen, Gorilla has abandoned all of their projects. There is a note about it on the readme. I needed a library like this and after some research, this one seemed to fit the build even though it is abandoned. I have forked the project and it can be found here: https://github.com/gopherlibs/feedhub

I would appreciate it if you were willing to recreate this PR over at my fork. Having looked at the PR here, I am confident we can get it merged into my fork.

p.s. You snagged the GitHub username magical? Nice.


          Merge branch 'main' into master

451fbce

pull-request-size bot added the size/M label

codecov bot commented Jul 30, 2023 •

edited

Loading

Codecov Report

Merging #83 (e9a32b9) into main (07a4f17) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #83   +/-   ##
=======================================
  Coverage   79.18%   79.18%           
=======================================
  Files           5        5           
  Lines         269      269           
=======================================
  Hits          213      213           
  Misses         48       48           
  Partials        8        8

Files Changed	Coverage Δ
atom.go	`92.42% <100.00%> (ø)`

milosgajdos approved these changes

View reviewed changes


          Merge branch 'main' into master

e9a32b9

coreydaley enabled auto-merge (squash)

August 17, 2023 01:29

coreydaley approved these changes

View reviewed changes

coreydaley merged commit 8b1dc44 into gorilla:main

Contributor Author

magical commented Aug 17, 2023

@coreydaley Thanks!

Bios-Marcel pushed a commit to Bios-Marcel/feeds that referenced this pull request


          Don't create an empty <summary> in Atom feeds (gorilla#83)

4af5d3a

When creating an AtomEntry from an Item, only set Summary if
item.Description is non-empty, like we already do for Content.

Currently we always emit a `<summary>` element, even if the item has a
non-empty `<content>` element. Having an empty `<summary>` in this case
may
be confusing for feed consumers.

The Atom RFC explicitly says that a `<summary>` is not required in
general.
(There are a couple special cases where it is required, but they aren't
relevant here.)

https://tools.ietf.org/html/rfc4287#section-4.1.1.1

Fixes gorilla#82

Co-authored-by: Corey Daley <cdaley@redhat.com>

jlelse pushed a commit to jlelse/feeds that referenced this pull request


          Don't create an empty <summary> in Atom feeds (gorilla#83)

7bc3665

When creating an AtomEntry from an Item, only set Summary if
item.Description is non-empty, like we already do for Content.

Currently we always emit a `<summary>` element, even if the item has a
non-empty `<content>` element. Having an empty `<summary>` in this case
may
be confusing for feed consumers.

The Atom RFC explicitly says that a `<summary>` is not required in
general.
(There are a couple special cases where it is required, but they aren't
relevant here.)

https://tools.ietf.org/html/rfc4287#section-4.1.1.1

Fixes gorilla#82

Co-authored-by: Corey Daley <cdaley@redhat.com>

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

Labels