Skip to content

Fixed: "Notice: Undefined variable: upgrade_notice ... "#4

Open
mikeschinkel wants to merge 2 commits into
markjaquith:masterfrom
mikeschinkel:master
Open

Fixed: "Notice: Undefined variable: upgrade_notice ... "#4
mikeschinkel wants to merge 2 commits into
markjaquith:masterfrom
mikeschinkel:master

Conversation

@mikeschinkel

Copy link
Copy Markdown

The variable $upgrade_notice is never initialized if there is no upgrade notice section which is the case with v1.0 plugins, so it throws the following error:

Notice: Undefined variable: upgrade_notice in
/Users/mikeschinkel/Tools/wppm/src/readme-parser/parse-readme.php on line 202

This patch simply moves the initialization of $upgrade_notice from inside the if( ... ) { ... } to just before it.

@mikeschinkel

Copy link
Copy Markdown
Author

Hey Mark,

Turns out there as another more serious issue; it did not handle License: <type> and License URI: <uri> so those values ended up in the 'short_description' array element. I added recognizers for both following your coding pattern and added their values to the returned array in my 2nd commit.

@markjaquith

Copy link
Copy Markdown
Owner

License and License URI probably shouldn't have been added to the example readme. Their utility and implementation is questionable. "GPL Version" would be more useful.

@mikeschinkel

Copy link
Copy Markdown
Author

Hi Mark,

Were you aware of this where @nacin said License and License URI were now used in the sample readme?

Anyway, I made my License and License URI update to this pull request before I noticed that tillkruess had made an earlier pull request also adding License and License URI. FWIW.

@markjaquith

Copy link
Copy Markdown
Owner

Ah, I'd forgotten it was @nacin who added it. Will have to think about that. As far as the repo goes, only GPL licenses are valid. Seems like overkill to have a URL field for one of two options.

@mikeschinkel

Copy link
Copy Markdown
Author

It's not a big deal to me either way as I ended up writing my own parser so I could have more control over the functionality, just thought I'd be nice and submit a pull request since I ran into readme.txt files that didn't work with yours.

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.

2 participants