Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #2299 - Missing setting descriptions in ACP #2526

Merged
merged 2 commits into from
Dec 7, 2016
Merged

Fixes #2299 - Missing setting descriptions in ACP #2526

merged 2 commits into from
Dec 7, 2016

Conversation

Eldenroot
Copy link
Contributor

@Eldenroot Eldenroot commented Nov 23, 2016

Fixes #2299 - Missing setting desctriptions in ACP

@JN-Jones @PaulBender @Destroy666x @dequeues

I am not sure about these: setting in L around n. 217 and 1411

Please review it carefully, I will try my best to find them all and fix the missing descriptions.

Fixes #2299 - Missing setting desctriptions in ACP
@Eldenroot Eldenroot changed the title Fixes #2299 - Missing setting desctriptions in ACP Fixes #2299 - Missing setting descriptions in ACP Nov 23, 2016
@JN-Jones
Copy link
Contributor

JN-Jones commented Dec 1, 2016

@Ben-Conway can you take a first look at this?

<disporder>1</disporder>
<optionscode><![CDATA[yesno]]></optionscode>
<settingvalue><![CDATA[1]]></settingvalue>
<isdefault>1</isdefault>
</setting>
<setting name="maxattachments">
<title>Maximum Attachments Per Post</title>
<description><![CDATA[The maximum number of attachments a user is allowed to upload per post.]]></description>
<description><![CDATA[The maximum number of attachments a user is allowed to upload per post. Set to 0 disable.]]></description>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Set to 0 disable." --> "Set to 0 to disable."

@Shade-
Copy link
Contributor

Shade- commented Dec 4, 2016

This looks good, except for the aforementioned part for which I requested a change. Note that I've only checked the diff report, some other descriptions may require editing with the "Set to 0 to disable" notice.

Btw, +1 to @Cu8eeeR for standardizing the Yes/No labels.

@Eldenroot
Copy link
Contributor Author

@Shade- thank you! I fixed the typo you mentioned above. It would be really nice if someone can look at this and find other places where the change is needed.

@euantorano
Copy link
Member

Looks good to me. I had a quick glance and didn't notice anywhere else needing this change.

@Ben-MyBB Ben-MyBB merged commit 7e6ca2c into mybb:feature Dec 7, 2016
@Ben-MyBB
Copy link
Member

Ben-MyBB commented Dec 7, 2016

Looks good to me.

@Eldenroot Eldenroot deleted the Fixes-2299 branch December 7, 2016 18:21
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.

Missing setting descriptions
5 participants