Make WordPress Core

Opened 14 years ago

Closed 13 years ago

#22140 closed enhancement (fixed)

fetch_feed parameter is documented incorrectly

Reported by: tlovett1 Owned by: nacin
Priority: normal Milestone: 3.6
Component: Inline Docs Version: 2.8
Severity: normal Keywords: has-patch commit
Cc: Focuses:

Description

The fetch_feed function in wp-includes/feed.php is documented to allow one parameter of type string. fetch_feed passes that parameter to the set_feed_url method of a newly created SimplePie object. However, set_feed_url can accept a URL string or an array of URL's. Therefore, fetch_feed should be documented as having one parameter of mixed type (string OR array).

Attachments (4)

fetch_feed_mixed_params.diff​ (1.1 KB ) - added by tlovett1 14 years ago.
A patch that updates the documentation for the fetch_feed method as well as renames the parameter.
22140.1.diff​ (1.2 KB ) - added by JustinSainton 14 years ago.
22140.2.diff​ (1.5 KB ) - added by JustinSainton 14 years ago.
22140.3.diff​ (1.3 KB ) - added by JustinSainton 14 years ago.

Download all attachments as: .zip

Change History (21)

@tlovett1
14 years ago

A patch that updates the documentation for the fetch_feed method as well as renames the parameter.

#1 @tlovett1
14 years ago

  • Summary fetch_feed parameter is misdocumented → fetch_feed parameter is documented incorrectly

#2 @taylorde
14 years ago

  • Cc td@… added

#3 @helenyhou
14 years ago

  • Component Feeds → Inline Docs

#4 @SergeyBiryukov
14 years ago

  • Version trunk → 2.8

#5 @helen
14 years ago

  • Milestone Awaiting Review → 3.6

#6 @JustinSainton
14 years ago

Thinking URLs doesn't need the apostrophe here?

#7 follow-up: @SergeyBiryukov
14 years ago

Not sure if we should advertise that $url can be an array here. See ​http://simplepie.org/wiki/faq/typical_multifeed_gotchas.

#8 in reply to: ↑ 7 @nacin
14 years ago

Replying to SergeyBiryukov:

Not sure if we should advertise that $url can be an array here. See ​http://simplepie.org/wiki/faq/typical_multifeed_gotchas.

I'd be inclined to agree as well.

#9 @SergeyBiryukov
14 years ago

  • Milestone 3.6
  • Resolution → wontfix
  • Status new → closed

#10 @rmccue
14 years ago

-1 on wontfix. Multifeeds are used so popularly that they're basically the main way to use SimplePie.

The only real gotchas with using them are that the feed-level accessors don't make sense, since you're operating on a group rather than on a single feed. For the next version of SimplePie, I'm ​working on making this clearer to users, but it's fairly obvious when you try and use them anyway.

#11 follow-up: @nacin
14 years ago

Okay, but let's tweak it a bit. Maybe something like:

@param mixed $url URL of feed to retrieve. If an array of URLs, the feeds are merged using SimplePie's multifeed feature. See also {@link ​http://simplepie.org/wiki/faq/typical_multifeed_gotchas}.

The "see also" is optional.

#12 @JustinSainton
14 years ago

Refreshed with nacin's suggestions.

#13 @ocean90
14 years ago

  • Milestone → 3.6
  • Resolution wontfix
  • Status closed → reopened

#14 in reply to: ↑ 11 @rmccue
14 years ago

Replying to nacin:

Okay, but let's tweak it a bit. Maybe something like:

@param mixed $url URL of feed to retrieve. If an array of URLs, the feeds are merged using SimplePie's multifeed feature. See also {@link ​http://simplepie.org/wiki/faq/typical_multifeed_gotchas}.

The "see also" is optional.

Sounds good.

Patch looks OK, but $url is used in the documentation, whereas $urls is used in the prototype. The see also has to also be on the same line (from memory).

#15 @JustinSainton
14 years ago

  • Keywords dev-feedback added

#16 @SergeyBiryukov
13 years ago

  • Keywords commit added; dev-feedback removed

#17 @nacin
13 years ago

  • Owner set to nacin
  • Resolution → fixed
  • Status reopened → closed

In 24054:

Document that fetch_feed() can accept multiple URLs, thus leveraging SimplePie's multifeed feature. props JustinSainton, fixes #22140.

Note: See TracTickets for help on using tickets.