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

Added number formatting to process queue ajax log content #125

Merged
merged 4 commits into from Jan 27, 2017

Conversation

Projects
None yet
3 participants
@samtuke
Contributor

samtuke commented Jan 9, 2017

No description provided.

@michield

This comment has been minimized.

Show comment
Hide comment
@michield

michield Jan 9, 2017

Member

did you test that ? Won't the %d turn it back into a plain integer?

Member

michield commented Jan 9, 2017

did you test that ? Won't the %d turn it back into a plain integer?

@bramley

This comment has been minimized.

Show comment
Hide comment
@bramley

bramley Jan 10, 2017

Contributor

It needs to use %s instead of %d

php -r 'printf("Sending in batches of %d messages\n", number_format(12345));'
Sending in batches of 12 messages
php -r 'printf("Sending in batches of %s messages\n", number_format(12345));'
Sending in batches of 12,345 messages
Contributor

bramley commented Jan 10, 2017

It needs to use %s instead of %d

php -r 'printf("Sending in batches of %d messages\n", number_format(12345));'
Sending in batches of 12 messages
php -r 'printf("Sending in batches of %s messages\n", number_format(12345));'
Sending in batches of 12,345 messages
@samtuke

This comment has been minimized.

Show comment
Hide comment
@samtuke

samtuke Jan 11, 2017

Contributor

Simple mistake, sorry, will fix.

Contributor

samtuke commented Jan 11, 2017

Simple mistake, sorry, will fix.

@samtuke samtuke closed this Jan 11, 2017

@samtuke

This comment has been minimized.

Show comment
Hide comment
@samtuke

samtuke Jan 27, 2017

Contributor

I believe the issues have been fixed with the new commits; reopening PR

Contributor

samtuke commented Jan 27, 2017

I believe the issues have been fixed with the new commits; reopening PR

@samtuke samtuke reopened this Jan 27, 2017

@michield

This comment has been minimized.

Show comment
Hide comment
@michield

michield Jan 27, 2017

Member

Hmm, I try to avoid changes like that, because it requires all translators to re-translate those strings. Some languages are not actively translated and would therefore "regress". It's tricky.

I'll do it this time, but we should be careful changing things too much, if it's not really essential, for the above reason.

Member

michield commented Jan 27, 2017

Hmm, I try to avoid changes like that, because it requires all translators to re-translate those strings. Some languages are not actively translated and would therefore "regress". It's tricky.

I'll do it this time, but we should be careful changing things too much, if it's not really essential, for the above reason.

@michield michield merged commit a523624 into phpList:master Jan 27, 2017

0 of 2 checks passed

continuous-integration/styleci/pr The StyleCI analysis has failed - 1 file needs addressing
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment