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

[Review 4] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() #1397

Merged
merged 1 commit into from
Sep 28, 2018

Conversation

andy-maier
Copy link
Contributor

@andy-maier andy-maier commented Sep 20, 2018

Note: This PR is on top of PR #1403.
For details, see the commit message (of the last commit).
Ready for review and merge.

@andy-maier andy-maier self-assigned this Sep 20, 2018
@andy-maier andy-maier added this to the 0.13.0 milestone Sep 20, 2018
@andy-maier andy-maier changed the title [WIP] Using new format strings in exceptions [WIP] Fixes #1072: Using new format strings in exceptions Sep 20, 2018
@andy-maier andy-maier force-pushed the andy/ascii-exceptions branch 2 times, most recently from b4f573a to 290a0f6 Compare September 20, 2018 21:05
@coveralls
Copy link

coveralls commented Sep 20, 2018

Coverage Status

Coverage increased (+0.04%) to 84.954% when pulling c90d7eb on andy/ascii-exceptions into 7a9ea73 on master.

@andy-maier andy-maier force-pushed the andy/ascii-exceptions branch 3 times, most recently from bad6958 to 48783fd Compare September 21, 2018 07:25
@andy-maier andy-maier changed the title [WIP] Fixes #1072: Using new format strings in exceptions [WIP] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() Sep 21, 2018
@andy-maier andy-maier force-pushed the andy/ascii-exceptions branch 3 times, most recently from c426a62 to 40a779a Compare September 23, 2018 17:04
@andy-maier andy-maier changed the title [WIP] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() [Review 3] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() Sep 23, 2018
@andy-maier andy-maier changed the title [Review 3] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() [Review 3] Partly fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() Sep 23, 2018
@andy-maier andy-maier changed the title [Review 3] Partly fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() [WIP] [Review 3] Partly fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() Sep 23, 2018
@andy-maier andy-maier changed the title [WIP] [Review 3] Partly fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() [WIP] [Review ] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() Sep 24, 2018
@andy-maier andy-maier changed the title [WIP] [Review ] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() [WIP] [Review 4] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() Sep 24, 2018
@andy-maier andy-maier changed the title [WIP] [Review 4] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() [Review 4] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() Sep 24, 2018
Copy link
Collaborator

@KSchopmeyer KSchopmeyer left a comment

Choose a reason for hiding this comment

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

I looked through a bunch of the changes. I also ran the tests and Pegasus tests and did not find anything. However, I did not review each and every change in all 20 something files.

@KSchopmeyer
Copy link
Collaborator

I could not find the changes.rst change to cover this.

Finally, does this mean that we really want to force all prints in the future to use format???

Details:

* Changed all usages of old-style formatting (i.e. using '%') to
  use new-style formatting with the new `_utils._format()` function.
  Usages of %r have been made ASCII-ensuring by the use of the new
  'A' conversion specifier. Some usages of %s have also been made
  ASCII-ensuring that way.

* Changed most usages of new-style formatting based on the str.format()
  function to use the new `_utils._format()` function. All usages of
  'r' conversion have been changed to use the new 'A' conversion.
  Some usages of 's' conversion (the default) have also been changed
  to use the new 'A' conversion, in order to ensure ASCII-only.

* Changed all implementations of `__repr__()` functions to use
  use new-style formatting with the new `_utils._format()` function
  and the new 'A' conversion specifier instead of '%r' or '!r'.

* Adjusted the expected test results intest_cim_obj.py accordingly.

* Now that the CIM object representations use the ASCII-only
  formatting, their repr() output is the same for py2 and py3.
  This allowed further simplifying test_recorder.py to get rid
  of the post-processing of the unicode indicator 'u'.

Signed-off-by: Andreas Maier <andreas.r.maier@gmx.de>
@andy-maier
Copy link
Contributor Author

I added a change log entry.

@andy-maier
Copy link
Contributor Author

As discussed in the call today, we would use new-style formatting for all future use. And also convert the old-style formatting to new-style in testsuite.

@andy-maier andy-maier merged commit 6fdc956 into master Sep 28, 2018
@andy-maier andy-maier deleted the andy/ascii-exceptions branch September 28, 2018 14:26
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.

None yet

3 participants