search

silverstripe-archive / silverstripe-forum

Forum module for SilverStripe CMS
http://silverstripe.org/forum-module
Other
34 stars 60 forks source link

FIX: Make subscribe selector more generic #183

Closed gordonbanderson closed 8 years ago

gordonbanderson commented 8 years ago

Fix for #182

camfindlay commented 8 years ago

@gordonbanderson thanks for these. Should have time tomorrow to look over. On an aside I/we need to check to make sure the master branch is in good shape compared to the 0.8 current stable. I think there was a few experiments that took place that we needed to revert/correct before a bit of progression on this module.

camfindlay commented 8 years ago

@dhensby do you remember where we got to with master branch on this module I remember we had a few things reverted etc. I wonder if we should simple make a copy of 0.8, cherry pick any thing the went on to master worth keeping and push up as the new master to move forward?

dhensby commented 8 years ago

@camfindlay I can't remember to be honest - all I recall is it being a complete mine-field and me thinking it needs a re-write ;)

camfindlay commented 8 years ago

All good @dhensby in that case would you be ok with my above decision to go back to drawing board on master branch? (I have a bit of time tomorrow NZDT to pick through the mess ;) )

gordonbanderson commented 8 years ago

Ah so are you saying the master branch is a mess...? I have the same fix applied to 0.8.1, would this be more suitable as a PR?

dhensby commented 8 years ago

All good @dhensby in that case would you be ok with my above decision to go back to drawing board on master branch? (I have a bit of time tomorrow NZDT to pick through the mess ;) )

yep :)

gordonbanderson commented 8 years ago

The user profile viewing and also registration is broken on the master branch so I've switched to 0.8 - worth reporting or not? If you are going to revamp/replace master I guess not.

camfindlay commented 8 years ago

@gordonbanderson if they are bug fixes only (and don't introduce a new feature or something that might cause breaks for people relying on APIs in 0.8 branch) then PRs against the 0.8 branch is the right place.

I'm going to be making a "develop" branch today that will eventually replace the current master branch (as we ended up getting that into a bit of a mess). This should allow us to move forward more easily.

camfindlay commented 8 years ago

A develop branch is up, cheery picked in some low hanging fruit, @dhensby I've left off you large template + structural change commit which was the one I think ended up causing grief a while back.

Seems still some failing tests on that branch which we'll want to resolve before this becomes new master.

camfindlay commented 8 years ago

Tests passing locally though.

Update... actually no they aren't - working on now.

camfindlay commented 8 years ago

Ok after half a day of battling this, here is what we've decided on #185

gordonbanderson commented 8 years ago

Fix up at https://github.com/silverstripe/silverstripe-forum/pull/187