search

Courseware / buddypress-courseware

BuddyPress Courseware
http://buddypress.coursewa.re
GNU General Public License v2.0
121 stars 34 forks source link

PHP Strict Standards, Notice and Warnings #121

Open lloc opened 10 years ago

lloc commented 10 years ago

Plugin generates some warnings and notices with PHP 5.5 and WP_DEBUG true when entering the admin screens.

christianwach commented 10 years ago

@lloc You might like to have a look at this branch on my fork: https://github.com/christianwach/buddypress-courseware/tree/cmw-merged I merged what I thought were the best bits of the other forks and fixed a whole heap of stuff on top of that. Unfortunately, I had to stop where I did due to time constraints.

lloc commented 10 years ago

Ah, I thought that this here might be the "main" repository.

https://twitter.com/jenmylo/status/518744634432831488

christianwach commented 10 years ago

It is the main repo. It's just that @iandunn, @imadalin and @sboisvert all added great code which has not been merged back in. I built on those forks to produce the branch on my fork. I never opened a PR, though, as this main repo seems unmaintained.

christianwach commented 10 years ago

Oh, I forgot to mention: from my inspection of the forks, it looked like @iandunn rewrote the plugin so that it does not require BuddyPress. It might be that fork which was designed to run on wp.org. I can't remember offhand whether my fork carries that work forward. UPDATE: it does contain the BP-mocking code.

christianwach commented 10 years ago

Either way, my branch will give you a considerable head-start.

christianwach commented 10 years ago

@lloc I just opened a PR so the branch is more easily discoverable.

Mamaduka commented 10 years ago

I think it would be better, if we won't drop BP as requirement. We might end-up writing a lot of "mock" BP code.

Not sure, if you all saw the tweet from Jen (https://twitter.com/jenmylo/status/518744634432831488), but current priority is to make sure plugin is compatible with latest version of WordPress and possible with latest BP.

Let's stick with this priority for now, we can cherry-pick commits, if any of those does what we need for now.

christianwach commented 10 years ago

@Mamaduka My branch is definitely compatible with BP 1.8, which was the state of play back in June. BP can be dropped as a requirement via a filter, but it's off by default. Happy to feed back on any of my code - if I can remember what it was for :) Look forward to this getting some TLC.

christianwach commented 10 years ago

Unfortunately, it looks like "imadalin" has closed their GitHub account, so all the refactoring that they did on this plugin (basically to update the PHP4-style class declarations and clean up whitespace) no longer exists as a fork. Seems like this commit is the only remaining evidence of it. There was a lot of work done for this.

stas commented 10 years ago

Hey guys, can somebody make a PR we can merge so that everyone can have a common place to start working on?

I know Ian started working on a BPless version, but I'm not sure what is the status of it. He emailed me a couple of months ago with a common issue which I will add as a ticket most likely.

I also know Mădălin started some work, but I never checked what is the status. I believe he's now under @16nsk. His fork seems to be https://github.com/16nsk/buddypress-courseware

I will be happy to review and merge branches that would first fix existing compatibility issues, after what we could figure out what features could be added/removed.

Thanks a lot for picking up the development.

christianwach commented 10 years ago

Ian Dunn's and Mădălin's forks basically had too many conflicts to maintain the commits from both, so I merged Mădălin's work in manually after applying Ian's commits. My own efforts begin thereafter at 543c7e0341692ecf054df2f93b375c6646696ac3 as you can see from the list on #123.

iandunn commented 10 years ago

I know Ian started working on a BPless version, but I'm not sure what is the status of it. He emailed me a couple of months ago with a common issue which I will add as a ticket most likely.

I haven't worked on it in a long time, but IIRC, it was mostly functional. I think it also had some compatibility fixes. It sounds like Christian has already incorporated it into his fork, but it's at https://github.com/iandunn/buddypress-courseware/tree/bp_not_required for anyone else who wants to take a look.

lloc commented 10 years ago

Interesting situation! Seems there is a lot of code... ;) Any ideas how to proceed to reach the first goal

make sure plugin is compatible with latest version of WordPress and possible with latest BP

... can we add all the enhancements after this?

christianwach commented 10 years ago

@lloc I don't remember any of the commits that I inspected (or added) being "enhancements" as such, unless you count BP being optional as an enhancement. IIRC Mădălin's major contribution was to update the classes and introduce the BPSP_Courseware_Component class. The code as I left it in June worked with the then-current version of BP. If you look at the diff between the master and cmw-merged branches, you'll see that there's not a lot in the way of new code - with a few exceptions it's mostly just cleaned up.

lloc commented 10 years ago

@christianwach OK, the term enhancement was probably not the best choice. ;) Let us find a pragmatic way to have a starting-point.