Skip to content

Conversation

@TheSerapher
Copy link
Contributor

Fixes #1921 once merged.

Fixes #1921 once merged.
@daygle
Copy link
Contributor

daygle commented Mar 10, 2014

Tested on my TGC pool.

I did find a possible issue, on the 'Pool workers' page the 'monitor' column is still displayed.

I know when you disable all notifications this column disappears.

@TheSerapher
Copy link
Contributor Author

Yeah will add that!

@TheSerapher
Copy link
Contributor Author

@daygle done, can you test again?

@daygle
Copy link
Contributor

daygle commented Mar 10, 2014

Unfortunately the column still displays...

@TheSerapher
Copy link
Contributor Author

Did you pull this commit when you updated: 4f01b27

I tried it out while adding it and it worked for me with different options. Can only try again tomorrow just trying to confirm you have that change.

@daygle
Copy link
Contributor

daygle commented Mar 10, 2014

Very weird? I trust that you have corrected it I'm just confused on why I'm having trouble.

I'm on git checkout disable-worker-notifications and have ensured that there are no custom templates.

@TheSerapher
Copy link
Contributor Author

Just check if that change I linked is actually added after a git pull. If yes I will have to check what i may have overlooked ;-)

@daygle
Copy link
Contributor

daygle commented Mar 10, 2014

I have checked the code for 'disable-worker-notifications' branch online and your changes don't appear in this repository.

The poolworkers/default.tpl page looks like it was last modified 2 months ago and doesn't include the fix.

@TheSerapher
Copy link
Contributor Author

You were talking about the admin page! Now I get it ;-) I removed it from the accounts worker page so people can't enable/disable the monitor anymore. I shall fix it on the admin page too then we should be good to go.

@daygle
Copy link
Contributor

daygle commented Mar 11, 2014

Haha now I see, we were both talking about different things. Oh well at least we are covering all areas :)

@TheSerapher
Copy link
Contributor Author

There we go, please test this again. Should be good to go now!

TheSerapher pushed a commit that referenced this pull request Mar 11, 2014
@daygle
Copy link
Contributor

daygle commented Mar 11, 2014

Perfect tested and everything is ok now. Thanks for the addition.

TheSerapher added a commit that referenced this pull request Mar 11, 2014
@TheSerapher TheSerapher merged commit 32ebfd8 into development Mar 11, 2014
@TheSerapher TheSerapher deleted the disable-worker-notifications branch March 11, 2014 08:32
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.

4 participants